diff options
306 files changed, 3242 insertions, 1552 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 19540e4331e..e4499b85fe1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.5-golang-1.8-git-2.13-chrome-62.0-node-8.x-yarn-1.2-postgresql-9.6" +image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.5-golang-1.8-git-2.14-chrome-63.0-node-8.x-yarn-1.2-postgresql-9.6" .default-cache: &default-cache key: "ruby-235-with-yarn" diff --git a/.rubocop.yml b/.rubocop.yml index c427f219a0d..7721cfaf850 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1185,7 +1185,20 @@ RSpec/SubjectStub: RSpec/VerifiedDoubles: Enabled: false -# GitlabSecurity ############################################################## +# Gitlab ################################################################### + +Gitlab/ModuleWithInstanceVariables: + Enable: true + Exclude: + # We ignore Rails helpers right now because it's hard to workaround it + - app/helpers/**/*_helper.rb + # We ignore Rails mailers right now because it's hard to workaround it + - app/mailers/emails/**/*.rb + # We ignore spec helpers because it usually doesn't matter + - spec/support/**/*.rb + - features/steps/**/*.rb + +# GitlabSecurity ########################################################### GitlabSecurity/DeepMunge: Enabled: true diff --git a/CHANGELOG.md b/CHANGELOG.md index adf097b52f3..fd7825b5f82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.2.5 (2017-12-15) + +### Fixed (8 changes) + +- Create a fork network for forks with a deleted source. !15595 +- Correctly link to a forked project from the new fork page. !15653 +- Fix the fork project functionality for projects with hashed storage. !15671 +- Fix updateEndpoint undefined error for issue_show app root. !15698 +- Fix broken illustration images for monitoring page empty states. !15889 +- Fix related branches/Merge requests failing to load when the hostname setting is changed. +- Fix gitlab:import:repos Rake task moving repositories into the wrong location. +- Gracefully handle case when repository's root ref does not exist. + +### Performance (3 changes) + +- Keep track of all circuitbreaker keys in a set. !15613 +- Only load branch names for protected branch checks. +- Optimize API /groups/:id/projects by preloading associations. + + ## 10.2.4 (2017-12-07) ### Security (5 changes) @@ -311,7 +311,7 @@ group :development, :test do gem 'fuubar', '~> 2.2.0' gem 'database_cleaner', '~> 1.5.0' - gem 'factory_girl_rails', '~> 4.7.0' + gem 'factory_bot_rails', '~> 4.8.2' gem 'rspec-rails', '~> 3.6.0' gem 'rspec-retry', '~> 0.4.5' gem 'spinach-rails', '~> 0.2.1' diff --git a/Gemfile.lock b/Gemfile.lock index 55600555e4d..11040fab805 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -195,10 +195,10 @@ GEM excon (0.57.1) execjs (2.6.0) expression_parser (0.9.0) - factory_girl (4.7.0) + factory_bot (4.8.2) activesupport (>= 3.0.0) - factory_girl_rails (4.7.0) - factory_girl (~> 4.7.0) + factory_bot_rails (4.8.2) + factory_bot (~> 4.8.2) railties (>= 3.0.0) faraday (0.12.2) multipart-post (>= 1.2, < 3) @@ -1020,7 +1020,7 @@ DEPENDENCIES dropzonejs-rails (~> 0.7.1) email_reply_trimmer (~> 0.1) email_spec (~> 1.6.0) - factory_girl_rails (~> 4.7.0) + factory_bot_rails (~> 4.8.2) faraday (~> 0.12) ffaker (~> 2.4) flay (~> 2.8.0) diff --git a/app/assets/javascripts/activities.js b/app/assets/javascripts/activities.js index f5f6b67f26e..6a0662ba903 100644 --- a/app/assets/javascripts/activities.js +++ b/app/assets/javascripts/activities.js @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign, class-methods-use-this */ -/* global Pager */ import Cookies from 'js-cookie'; +import Pager from './pager'; import { localTimeAgo } from './lib/utils/datetime_utility'; export default class Activities { diff --git a/app/assets/javascripts/behaviors/secret_values.js b/app/assets/javascripts/behaviors/secret_values.js new file mode 100644 index 00000000000..1cf0b960eb0 --- /dev/null +++ b/app/assets/javascripts/behaviors/secret_values.js @@ -0,0 +1,42 @@ +import { n__ } from '../locale'; +import { convertPermissionToBoolean } from '../lib/utils/common_utils'; + +export default class SecretValues { + constructor(container) { + this.container = container; + } + + init() { + this.values = this.container.querySelectorAll('.js-secret-value'); + this.placeholders = this.container.querySelectorAll('.js-secret-value-placeholder'); + this.revealButton = this.container.querySelector('.js-secret-value-reveal-button'); + + this.revealText = n__('Reveal value', 'Reveal values', this.values.length); + this.hideText = n__('Hide value', 'Hide values', this.values.length); + + const isRevealed = convertPermissionToBoolean(this.revealButton.dataset.secretRevealStatus); + this.updateDom(isRevealed); + + this.revealButton.addEventListener('click', this.onRevealButtonClicked.bind(this)); + } + + onRevealButtonClicked() { + const previousIsRevealed = convertPermissionToBoolean( + this.revealButton.dataset.secretRevealStatus, + ); + this.updateDom(!previousIsRevealed); + } + + updateDom(isRevealed) { + this.values.forEach((value) => { + value.classList.toggle('hide', !isRevealed); + }); + + this.placeholders.forEach((placeholder) => { + placeholder.classList.toggle('hide', isRevealed); + }); + + this.revealButton.textContent = isRevealed ? this.hideText : this.revealText; + this.revealButton.dataset.secretRevealStatus = isRevealed; + } +} diff --git a/app/assets/javascripts/commits.js b/app/assets/javascripts/commits.js index be58392135c..3a03cbf6b90 100644 --- a/app/assets/javascripts/commits.js +++ b/app/assets/javascripts/commits.js @@ -1,10 +1,10 @@ /* eslint-disable func-names, wrap-iife, consistent-return, no-return-assign, no-param-reassign, one-var-declaration-per-line, no-unused-vars, prefer-template, object-shorthand, prefer-arrow-callback */ -/* global Pager */ import { pluralize } from './lib/utils/text_utility'; import { localTimeAgo } from './lib/utils/datetime_utility'; +import Pager from './pager'; export default (function () { const CommitsList = {}; diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 522f5d12b30..f62a0208110 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -7,8 +7,8 @@ import IssuableForm from './issuable_form'; import LabelsSelect from './labels_select'; /* global MilestoneSelect */ import NewBranchForm from './new_branch_form'; -/* global NotificationsForm */ -/* global NotificationsDropdown */ +import NotificationsForm from './notifications_form'; +import notificationsDropdown from './notifications_dropdown'; import groupAvatar from './group_avatar'; import GroupLabelSubscription from './group_label_subscription'; /* global LineHighlighter */ @@ -35,6 +35,7 @@ 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 DeleteModal from './branches/branches_delete_modal'; import Group from './group'; import GroupsList from './groups_list'; @@ -90,7 +91,6 @@ import memberExpirationDate from './member_expiration_date'; import DueDateSelectors from './due_date_select'; import Diff from './diff'; import ProjectLabelSubscription from './project_label_subscription'; -import ProjectVariables from './project_variables'; import SearchAutocomplete from './search_autocomplete'; import Activities from './activities'; @@ -414,7 +414,7 @@ import Activities from './activities'; const newGroupChildWrapper = document.querySelector('.js-new-project-subgroup'); shortcut_handler = new ShortcutsNavigation(); new NotificationsForm(); - new NotificationsDropdown(); + notificationsDropdown(); new ProjectsList(); if (newGroupChildWrapper) { @@ -527,8 +527,18 @@ import Activities from './activities'; case 'projects:settings:ci_cd:show': // Initialize expandable settings panels initSettingsPanels(); + + const runnerToken = document.querySelector('.js-secret-runner-token'); + if (runnerToken) { + const runnerTokenSecretValue = new SecretValues(runnerToken); + runnerTokenSecretValue.init(); + } case 'groups:settings:ci_cd:show': - new ProjectVariables(); + const secretVariableTable = document.querySelector('.js-secret-variable-table'); + if (secretVariableTable) { + const secretVariableTableValues = new SecretValues(secretVariableTable); + secretVariableTableValues.init(); + } break; case 'ci:lints:create': case 'ci:lints:show': @@ -617,7 +627,7 @@ import Activities from './activities'; break; case 'profiles': new NotificationsForm(); - new NotificationsDropdown(); + notificationsDropdown(); break; case 'projects': new Project(); @@ -640,7 +650,7 @@ import Activities from './activities'; case 'show': new Star(); new ProjectNew(); - new NotificationsDropdown(); + notificationsDropdown(); break; case 'wikis': new Wikis(); diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 96284c4c168..ae3f76873cf 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -34,16 +34,11 @@ import { getLocationHash, visitUrl } from './lib/utils/url_utility'; import './behaviors/'; // everything else -import './activities'; -import './admin'; import loadAwardsHandler from './awards_handler'; import bp from './breakpoints'; import './confirm_danger_modal'; import Flash, { removeFlashClickListener } from './flash'; import './gl_dropdown'; -import './gl_field_error'; -import './gl_field_errors'; -import './gl_form'; import initTodoToggle from './header'; import initImporterStatus from './importer_status'; import './layout_nav'; @@ -54,11 +49,7 @@ import './merge_request'; import './merge_request_tabs'; import './milestone_select'; import './notes'; -import './notifications_dropdown'; -import './notifications_form'; -import './pager'; import './preview_markdown'; -import './project_import'; import './projects_dropdown'; import './render_gfm'; import './right_sidebar'; diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index a9c08df4f93..6946c0b30f0 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -3,7 +3,7 @@ import 'vendor/jquery.waitforimages'; import TaskList from './task_list'; -import './merge_request_tabs'; +import MergeRequestTabs from './merge_request_tabs'; import IssuablesHelper from './helpers/issuables_helper'; import { addDelimiter } from './lib/utils/text_utility'; @@ -51,7 +51,7 @@ import { addDelimiter } from './lib/utils/text_utility'; if (window.mrTabs) { window.mrTabs.unbindEvents(); } - window.mrTabs = new gl.MergeRequestTabs(this.opts); + window.mrTabs = new MergeRequestTabs(this.opts); }; MergeRequest.prototype.showAllCommits = function() { diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index de84e28f915..cacca35ca98 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -63,387 +63,382 @@ import syntaxHighlight from './syntax_highlight'; // /* eslint-enable max-len */ -(() => { - // Store the `location` object, allowing for easier stubbing in tests - let location = window.location; - - class MergeRequestTabs { - - constructor({ action, setUrl, stubLocation } = {}) { - const mergeRequestTabs = document.querySelector('.js-tabs-affix'); - const navbar = document.querySelector('.navbar-gitlab'); - const paddingTop = 16; - - this.diffsLoaded = false; - this.pipelinesLoaded = false; - this.commitsLoaded = false; - this.fixedLayoutPref = null; - - this.setUrl = setUrl !== undefined ? setUrl : true; - this.setCurrentAction = this.setCurrentAction.bind(this); - this.tabShown = this.tabShown.bind(this); - this.showTab = this.showTab.bind(this); - this.stickyTop = navbar ? navbar.offsetHeight - paddingTop : 0; - - if (mergeRequestTabs) { - this.stickyTop += mergeRequestTabs.offsetHeight; - } +// Store the `location` object, allowing for easier stubbing in tests +let location = window.location; - if (stubLocation) { - location = stubLocation; - } +export default class MergeRequestTabs { - this.bindEvents(); - this.activateTab(action); - this.initAffix(); - } + constructor({ action, setUrl, stubLocation } = {}) { + const mergeRequestTabs = document.querySelector('.js-tabs-affix'); + const navbar = document.querySelector('.navbar-gitlab'); + const paddingTop = 16; - bindEvents() { - $(document) - .on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown) - .on('click', '.js-show-tab', this.showTab); + this.diffsLoaded = false; + this.pipelinesLoaded = false; + this.commitsLoaded = false; + this.fixedLayoutPref = null; - $('.merge-request-tabs a[data-toggle="tab"]') - .on('click', this.clickTab); - } + this.setUrl = setUrl !== undefined ? setUrl : true; + this.setCurrentAction = this.setCurrentAction.bind(this); + this.tabShown = this.tabShown.bind(this); + this.showTab = this.showTab.bind(this); + this.stickyTop = navbar ? navbar.offsetHeight - paddingTop : 0; - // Used in tests - unbindEvents() { - $(document) - .off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown) - .off('click', '.js-show-tab', this.showTab); + if (mergeRequestTabs) { + this.stickyTop += mergeRequestTabs.offsetHeight; + } - $('.merge-request-tabs a[data-toggle="tab"]') - .off('click', this.clickTab); + if (stubLocation) { + location = stubLocation; } - destroyPipelinesView() { - if (this.commitPipelinesTable) { - this.commitPipelinesTable.$destroy(); - this.commitPipelinesTable = null; + this.bindEvents(); + this.activateTab(action); + this.initAffix(); + } - document.querySelector('#commit-pipeline-table-view').innerHTML = ''; - } - } + bindEvents() { + $(document) + .on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown) + .on('click', '.js-show-tab', this.showTab); - showTab(e) { - e.preventDefault(); - this.activateTab($(e.target).data('action')); - } + $('.merge-request-tabs a[data-toggle="tab"]') + .on('click', this.clickTab); + } - clickTab(e) { - if (e.currentTarget && isMetaClick(e)) { - const targetLink = e.currentTarget.getAttribute('href'); - e.stopImmediatePropagation(); - e.preventDefault(); - window.open(targetLink, '_blank'); - } + // Used in tests + unbindEvents() { + $(document) + .off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown) + .off('click', '.js-show-tab', this.showTab); + + $('.merge-request-tabs a[data-toggle="tab"]') + .off('click', this.clickTab); + } + + destroyPipelinesView() { + if (this.commitPipelinesTable) { + this.commitPipelinesTable.$destroy(); + this.commitPipelinesTable = null; + + document.querySelector('#commit-pipeline-table-view').innerHTML = ''; } + } - tabShown(e) { - const $target = $(e.target); - const action = $target.data('action'); + showTab(e) { + e.preventDefault(); + this.activateTab($(e.target).data('action')); + } - if (action === 'commits') { - this.loadCommits($target.attr('href')); - this.expandView(); - this.resetViewContainer(); - this.destroyPipelinesView(); - } else if (this.isDiffAction(action)) { - this.loadDiff($target.attr('href')); - if (bp.getBreakpointSize() !== 'lg') { - this.shrinkView(); - } - if (this.diffViewType() === 'parallel') { - this.expandViewContainer(); - } - this.destroyPipelinesView(); - } else if (action === 'pipelines') { - this.resetViewContainer(); - this.mountPipelinesView(); - } else { - if (bp.getBreakpointSize() !== 'xs') { - this.expandView(); - } - this.resetViewContainer(); - this.destroyPipelinesView(); + clickTab(e) { + if (e.currentTarget && isMetaClick(e)) { + const targetLink = e.currentTarget.getAttribute('href'); + e.stopImmediatePropagation(); + e.preventDefault(); + window.open(targetLink, '_blank'); + } + } - initDiscussionTab(); + tabShown(e) { + const $target = $(e.target); + const action = $target.data('action'); + + if (action === 'commits') { + this.loadCommits($target.attr('href')); + this.expandView(); + this.resetViewContainer(); + this.destroyPipelinesView(); + } else if (this.isDiffAction(action)) { + this.loadDiff($target.attr('href')); + if (bp.getBreakpointSize() !== 'lg') { + this.shrinkView(); } - if (this.setUrl) { - this.setCurrentAction(action); + if (this.diffViewType() === 'parallel') { + this.expandViewContainer(); } + this.destroyPipelinesView(); + } else if (action === 'pipelines') { + this.resetViewContainer(); + this.mountPipelinesView(); + } else { + if (bp.getBreakpointSize() !== 'xs') { + this.expandView(); + } + this.resetViewContainer(); + this.destroyPipelinesView(); + + initDiscussionTab(); + } + if (this.setUrl) { + this.setCurrentAction(action); } + } - scrollToElement(container) { - if (location.hash) { - const offset = 0 - ( - $('.navbar-gitlab').outerHeight() + - $('.js-tabs-affix').outerHeight() - ); - const $el = $(`${container} ${location.hash}:not(.match)`); - if ($el.length) { - $.scrollTo($el[0], { offset }); - } + scrollToElement(container) { + if (location.hash) { + const offset = 0 - ( + $('.navbar-gitlab').outerHeight() + + $('.js-tabs-affix').outerHeight() + ); + const $el = $(`${container} ${location.hash}:not(.match)`); + if ($el.length) { + $.scrollTo($el[0], { offset }); } } + } + + // Activate a tab based on the current action + activateTab(action) { + // important note: the .tab('show') method triggers 'shown.bs.tab' event itself + $(`.merge-request-tabs a[data-action='${action}']`).tab('show'); + } - // Activate a tab based on the current action - activateTab(action) { - // important note: the .tab('show') method triggers 'shown.bs.tab' event itself - $(`.merge-request-tabs a[data-action='${action}']`).tab('show'); + // Replaces the current Merge Request-specific action in the URL with a new one + // + // If the action is "notes", the URL is reset to the standard + // `MergeRequests#show` route. + // + // Examples: + // + // location.pathname # => "/namespace/project/merge_requests/1" + // setCurrentAction('diffs') + // location.pathname # => "/namespace/project/merge_requests/1/diffs" + // + // location.pathname # => "/namespace/project/merge_requests/1/diffs" + // setCurrentAction('show') + // location.pathname # => "/namespace/project/merge_requests/1" + // + // location.pathname # => "/namespace/project/merge_requests/1/diffs" + // setCurrentAction('commits') + // location.pathname # => "/namespace/project/merge_requests/1/commits" + // + // Returns the new URL String + setCurrentAction(action) { + this.currentAction = action; + + // Remove a trailing '/commits' '/diffs' '/pipelines' + let newState = location.pathname.replace(/\/(commits|diffs|pipelines)(\.html)?\/?$/, ''); + + // Append the new action if we're on a tab other than 'notes' + if (this.currentAction !== 'show' && this.currentAction !== 'new') { + newState += `/${this.currentAction}`; } - // Replaces the current Merge Request-specific action in the URL with a new one - // - // If the action is "notes", the URL is reset to the standard - // `MergeRequests#show` route. - // - // Examples: - // - // location.pathname # => "/namespace/project/merge_requests/1" - // setCurrentAction('diffs') - // location.pathname # => "/namespace/project/merge_requests/1/diffs" - // - // location.pathname # => "/namespace/project/merge_requests/1/diffs" - // setCurrentAction('show') - // location.pathname # => "/namespace/project/merge_requests/1" - // - // location.pathname # => "/namespace/project/merge_requests/1/diffs" - // setCurrentAction('commits') - // location.pathname # => "/namespace/project/merge_requests/1/commits" - // - // Returns the new URL String - setCurrentAction(action) { - this.currentAction = action; + // Ensure parameters and hash come along for the ride + newState += location.search + location.hash; - // Remove a trailing '/commits' '/diffs' '/pipelines' - let newState = location.pathname.replace(/\/(commits|diffs|pipelines)(\.html)?\/?$/, ''); + // TODO: Consider refactoring in light of turbolinks removal. - // Append the new action if we're on a tab other than 'notes' - if (this.currentAction !== 'show' && this.currentAction !== 'new') { - newState += `/${this.currentAction}`; - } + // Replace the current history state with the new one without breaking + // Turbolinks' history. + // + // See https://github.com/rails/turbolinks/issues/363 + window.history.replaceState({ + url: newState, + }, document.title, newState); - // Ensure parameters and hash come along for the ride - newState += location.search + location.hash; + return newState; + } - // TODO: Consider refactoring in light of turbolinks removal. + loadCommits(source) { + if (this.commitsLoaded) { + return; + } + this.ajaxGet({ + url: `${source}.json`, + success: (data) => { + document.querySelector('div#commits').innerHTML = data.html; + localTimeAgo($('.js-timeago', 'div#commits')); + this.commitsLoaded = true; + this.scrollToElement('#commits'); + }, + }); + } - // Replace the current history state with the new one without breaking - // Turbolinks' history. - // - // See https://github.com/rails/turbolinks/issues/363 - window.history.replaceState({ - url: newState, - }, document.title, newState); + mountPipelinesView() { + const pipelineTableViewEl = document.querySelector('#commit-pipeline-table-view'); + const CommitPipelinesTable = gl.CommitPipelinesTable; + this.commitPipelinesTable = new CommitPipelinesTable({ + propsData: { + endpoint: pipelineTableViewEl.dataset.endpoint, + helpPagePath: pipelineTableViewEl.dataset.helpPagePath, + emptyStateSvgPath: pipelineTableViewEl.dataset.emptyStateSvgPath, + errorStateSvgPath: pipelineTableViewEl.dataset.errorStateSvgPath, + autoDevopsHelpPath: pipelineTableViewEl.dataset.helpAutoDevopsPath, + }, + }).$mount(); + + // $mount(el) replaces the el with the new rendered component. We need it in order to mount + // it everytime this tab is clicked - https://vuejs.org/v2/api/#vm-mount + pipelineTableViewEl.appendChild(this.commitPipelinesTable.$el); + } - return newState; + loadDiff(source) { + if (this.diffsLoaded) { + document.dispatchEvent(new CustomEvent('scroll')); + return; } - loadCommits(source) { - if (this.commitsLoaded) { - return; - } - this.ajaxGet({ - url: `${source}.json`, - success: (data) => { - document.querySelector('div#commits').innerHTML = data.html; - localTimeAgo($('.js-timeago', 'div#commits')); - this.commitsLoaded = true; - this.scrollToElement('#commits'); - }, - }); - } + // We extract pathname for the current Changes tab anchor href + // some pages like MergeRequestsController#new has query parameters on that anchor + const urlPathname = parseUrlPathname(source); - mountPipelinesView() { - const pipelineTableViewEl = document.querySelector('#commit-pipeline-table-view'); - const CommitPipelinesTable = gl.CommitPipelinesTable; - this.commitPipelinesTable = new CommitPipelinesTable({ - propsData: { - endpoint: pipelineTableViewEl.dataset.endpoint, - helpPagePath: pipelineTableViewEl.dataset.helpPagePath, - emptyStateSvgPath: pipelineTableViewEl.dataset.emptyStateSvgPath, - errorStateSvgPath: pipelineTableViewEl.dataset.errorStateSvgPath, - autoDevopsHelpPath: pipelineTableViewEl.dataset.helpAutoDevopsPath, - }, - }).$mount(); + this.ajaxGet({ + url: `${urlPathname}.json${location.search}`, + success: (data) => { + const $container = $('#diffs'); + $container.html(data.html); - // $mount(el) replaces the el with the new rendered component. We need it in order to mount - // it everytime this tab is clicked - https://vuejs.org/v2/api/#vm-mount - pipelineTableViewEl.appendChild(this.commitPipelinesTable.$el); - } + initChangesDropdown(this.stickyTop); - loadDiff(source) { - if (this.diffsLoaded) { - document.dispatchEvent(new CustomEvent('scroll')); - return; - } + if (typeof gl.diffNotesCompileComponents !== 'undefined') { + gl.diffNotesCompileComponents(); + } + + localTimeAgo($('.js-timeago', 'div#diffs')); + syntaxHighlight($('#diffs .js-syntax-highlight')); - // We extract pathname for the current Changes tab anchor href - // some pages like MergeRequestsController#new has query parameters on that anchor - const urlPathname = parseUrlPathname(source); - - this.ajaxGet({ - url: `${urlPathname}.json${location.search}`, - success: (data) => { - const $container = $('#diffs'); - $container.html(data.html); - - initChangesDropdown(this.stickyTop); - - if (typeof gl.diffNotesCompileComponents !== 'undefined') { - gl.diffNotesCompileComponents(); - } - - localTimeAgo($('.js-timeago', 'div#diffs')); - syntaxHighlight($('#diffs .js-syntax-highlight')); - - if (this.diffViewType() === 'parallel' && this.isDiffAction(this.currentAction)) { - this.expandViewContainer(); - } - this.diffsLoaded = true; - - new Diff(); - this.scrollToElement('#diffs'); - - $('.diff-file').each((i, el) => { - new BlobForkSuggestion({ - openButtons: $(el).find('.js-edit-blob-link-fork-toggler'), - forkButtons: $(el).find('.js-fork-suggestion-button'), - cancelButtons: $(el).find('.js-cancel-fork-suggestion-button'), - suggestionSections: $(el).find('.js-file-fork-suggestion-section'), - actionTextPieces: $(el).find('.js-file-fork-suggestion-section-action'), - }) - .init(); + if (this.diffViewType() === 'parallel' && this.isDiffAction(this.currentAction)) { + this.expandViewContainer(); + } + this.diffsLoaded = true; + + new Diff(); + this.scrollToElement('#diffs'); + + $('.diff-file').each((i, el) => { + new BlobForkSuggestion({ + openButtons: $(el).find('.js-edit-blob-link-fork-toggler'), + forkButtons: $(el).find('.js-fork-suggestion-button'), + cancelButtons: $(el).find('.js-cancel-fork-suggestion-button'), + suggestionSections: $(el).find('.js-file-fork-suggestion-section'), + actionTextPieces: $(el).find('.js-file-fork-suggestion-section-action'), + }) + .init(); + }); + + // Scroll any linked note into view + // Similar to `toggler_behavior` in the discussion tab + const hash = getLocationHash(); + const anchor = hash && $container.find(`.note[id="${hash}"]`); + if (anchor && anchor.length > 0) { + const notesContent = anchor.closest('.notes_content'); + const lineType = notesContent.hasClass('new') ? 'new' : 'old'; + notes.toggleDiffNote({ + target: anchor, + lineType, + forceShow: true, }); + anchor[0].scrollIntoView(); + handleLocationHash(); + // We have multiple elements on the page with `#note_xxx` + // (discussion and diff tabs) and `:target` only applies to the first + anchor.addClass('target'); + } + }, + }); + } - // Scroll any linked note into view - // Similar to `toggler_behavior` in the discussion tab - const hash = getLocationHash(); - const anchor = hash && $container.find(`.note[id="${hash}"]`); - if (anchor && anchor.length > 0) { - const notesContent = anchor.closest('.notes_content'); - const lineType = notesContent.hasClass('new') ? 'new' : 'old'; - notes.toggleDiffNote({ - target: anchor, - lineType, - forceShow: true, - }); - anchor[0].scrollIntoView(); - handleLocationHash(); - // We have multiple elements on the page with `#note_xxx` - // (discussion and diff tabs) and `:target` only applies to the first - anchor.addClass('target'); - } - }, - }); - } + // Show or hide the loading spinner + // + // status - Boolean, true to show, false to hide + toggleLoading(status) { + $('.mr-loading-status .loading').toggle(status); + } - // Show or hide the loading spinner - // - // status - Boolean, true to show, false to hide - toggleLoading(status) { - $('.mr-loading-status .loading').toggle(status); - } + ajaxGet(options) { + const defaults = { + beforeSend: () => this.toggleLoading(true), + error: () => new Flash('An error occurred while fetching this tab.', 'alert'), + complete: () => this.toggleLoading(false), + dataType: 'json', + type: 'GET', + }; + $.ajax($.extend({}, defaults, options)); + } - ajaxGet(options) { - const defaults = { - beforeSend: () => this.toggleLoading(true), - error: () => new Flash('An error occurred while fetching this tab.', 'alert'), - complete: () => this.toggleLoading(false), - dataType: 'json', - type: 'GET', - }; - $.ajax($.extend({}, defaults, options)); - } + diffViewType() { + return $('.inline-parallel-buttons a.active').data('view-type'); + } - diffViewType() { - return $('.inline-parallel-buttons a.active').data('view-type'); - } + isDiffAction(action) { + return action === 'diffs' || action === 'new/diffs'; + } - isDiffAction(action) { - return action === 'diffs' || action === 'new/diffs'; + expandViewContainer() { + const $wrapper = $('.content-wrapper .container-fluid').not('.breadcrumbs'); + if (this.fixedLayoutPref === null) { + this.fixedLayoutPref = $wrapper.hasClass('container-limited'); } + $wrapper.removeClass('container-limited'); + } - expandViewContainer() { - const $wrapper = $('.content-wrapper .container-fluid').not('.breadcrumbs'); - if (this.fixedLayoutPref === null) { - this.fixedLayoutPref = $wrapper.hasClass('container-limited'); - } - $wrapper.removeClass('container-limited'); + resetViewContainer() { + if (this.fixedLayoutPref !== null) { + $('.content-wrapper .container-fluid') + .toggleClass('container-limited', this.fixedLayoutPref); } + } - resetViewContainer() { - if (this.fixedLayoutPref !== null) { - $('.content-wrapper .container-fluid') - .toggleClass('container-limited', this.fixedLayoutPref); - } - } + shrinkView() { + const $gutterIcon = $('.js-sidebar-toggle i:visible'); - shrinkView() { - const $gutterIcon = $('.js-sidebar-toggle i:visible'); + // Wait until listeners are set + setTimeout(() => { + // Only when sidebar is expanded + if ($gutterIcon.is('.fa-angle-double-right')) { + $gutterIcon.closest('a').trigger('click', [true]); + } + }, 0); + } - // Wait until listeners are set - setTimeout(() => { - // Only when sidebar is expanded - if ($gutterIcon.is('.fa-angle-double-right')) { - $gutterIcon.closest('a').trigger('click', [true]); - } - }, 0); + // Expand the issuable sidebar unless the user explicitly collapsed it + expandView() { + if (Cookies.get('collapsed_gutter') === 'true') { + return; } + const $gutterIcon = $('.js-sidebar-toggle i:visible'); - // Expand the issuable sidebar unless the user explicitly collapsed it - expandView() { - if (Cookies.get('collapsed_gutter') === 'true') { - return; + // Wait until listeners are set + setTimeout(() => { + // Only when sidebar is collapsed + if ($gutterIcon.is('.fa-angle-double-left')) { + $gutterIcon.closest('a').trigger('click', [true]); } - const $gutterIcon = $('.js-sidebar-toggle i:visible'); + }, 0); + } - // Wait until listeners are set - setTimeout(() => { - // Only when sidebar is collapsed - if ($gutterIcon.is('.fa-angle-double-left')) { - $gutterIcon.closest('a').trigger('click', [true]); - } - }, 0); - } + initAffix() { + const $tabs = $('.js-tabs-affix'); + const $fixedNav = $('.navbar-gitlab'); + + // Screen space on small screens is usually very sparse + // So we dont affix the tabs on these + if (bp.getBreakpointSize() === 'xs' || !$tabs.length) return; + + /** + If the browser does not support position sticky, it returns the position as static. + If the browser does support sticky, then we allow the browser to handle it, if not + then we default back to Bootstraps affix + **/ + if ($tabs.css('position') !== 'static') return; + + const $diffTabs = $('#diff-notes-app'); + + $tabs.off('affix.bs.affix affix-top.bs.affix') + .affix({ + offset: { + top: () => ( + $diffTabs.offset().top - $tabs.height() - $fixedNav.height() + ), + }, + }) + .on('affix.bs.affix', () => $diffTabs.css({ marginTop: $tabs.height() })) + .on('affix-top.bs.affix', () => $diffTabs.css({ marginTop: '' })); - initAffix() { - const $tabs = $('.js-tabs-affix'); - const $fixedNav = $('.navbar-gitlab'); - - // Screen space on small screens is usually very sparse - // So we dont affix the tabs on these - if (bp.getBreakpointSize() === 'xs' || !$tabs.length) return; - - /** - If the browser does not support position sticky, it returns the position as static. - If the browser does support sticky, then we allow the browser to handle it, if not - then we default back to Bootstraps affix - **/ - if ($tabs.css('position') !== 'static') return; - - const $diffTabs = $('#diff-notes-app'); - - $tabs.off('affix.bs.affix affix-top.bs.affix') - .affix({ - offset: { - top: () => ( - $diffTabs.offset().top - $tabs.height() - $fixedNav.height() - ), - }, - }) - .on('affix.bs.affix', () => $diffTabs.css({ marginTop: $tabs.height() })) - .on('affix-top.bs.affix', () => $diffTabs.css({ marginTop: '' })); - - // Fix bug when reloading the page already scrolling - if ($tabs.hasClass('affix')) { - $tabs.trigger('affix.bs.affix'); - } + // Fix bug when reloading the page already scrolling + if ($tabs.hasClass('affix')) { + $tabs.trigger('affix.bs.affix'); } } - - window.gl = window.gl || {}; - window.gl.MergeRequestTabs = MergeRequestTabs; -})(); +} diff --git a/app/assets/javascripts/notifications_dropdown.js b/app/assets/javascripts/notifications_dropdown.js index f90ac2d9f71..9570d1c00aa 100644 --- a/app/assets/javascripts/notifications_dropdown.js +++ b/app/assets/javascripts/notifications_dropdown.js @@ -1,31 +1,25 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, one-var, no-var, one-var-declaration-per-line, no-unused-vars, consistent-return, prefer-arrow-callback, no-else-return, max-len */ import Flash from './flash'; -(function() { - this.NotificationsDropdown = (function() { - function NotificationsDropdown() { - $(document).off('click', '.update-notification').on('click', '.update-notification', function(e) { - var form, label, notificationLevel; - e.preventDefault(); - if ($(this).is('.is-active') && $(this).data('notification-level') === 'custom') { - return; - } - notificationLevel = $(this).data('notification-level'); - label = $(this).data('notification-title'); - form = $(this).parents('.notification-form:first'); - form.find('.js-notification-loading').toggleClass('fa-bell fa-spin fa-spinner'); - form.find('#notification_setting_level').val(notificationLevel); - return form.submit(); - }); - $(document).off('ajax:success', '.notification-form').on('ajax:success', '.notification-form', function(e, data) { - if (data.saved) { - return $(e.currentTarget).closest('.js-notification-dropdown').replaceWith(data.html); - } else { - return new Flash('Failed to save new settings', 'alert'); - } - }); +export default function notificationsDropdown() { + $(document).on('click', '.update-notification', function updateNotificationCallback(e) { + e.preventDefault(); + if ($(this).is('.is-active') && $(this).data('notification-level') === 'custom') { + return; } - return NotificationsDropdown; - })(); -}).call(window); + const notificationLevel = $(this).data('notification-level'); + const form = $(this).parents('.notification-form:first'); + + form.find('.js-notification-loading').toggleClass('fa-bell fa-spin fa-spinner'); + form.find('#notification_setting_level').val(notificationLevel); + form.submit(); + }); + + $(document).on('ajax:success', '.notification-form', (e, data) => { + if (data.saved) { + $(e.currentTarget).closest('.js-notification-dropdown').replaceWith(data.html); + } else { + Flash('Failed to save new settings', 'alert'); + } + }); +} diff --git a/app/assets/javascripts/notifications_form.js b/app/assets/javascripts/notifications_form.js index 2ab9c4fed2c..4534360d577 100644 --- a/app/assets/javascripts/notifications_form.js +++ b/app/assets/javascripts/notifications_form.js @@ -1,55 +1,50 @@ -/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, one-var, one-var-declaration-per-line, newline-per-chained-call, comma-dangle, consistent-return, prefer-arrow-callback, max-len */ -(function() { - this.NotificationsForm = (function() { - function NotificationsForm() { - this.toggleCheckbox = this.toggleCheckbox.bind(this); - this.removeEventListeners(); - this.initEventListeners(); - } +export default class NotificationsForm { + constructor() { + this.toggleCheckbox = this.toggleCheckbox.bind(this); + this.initEventListeners(); + } - NotificationsForm.prototype.removeEventListeners = function() { - return $(document).off('change', '.js-custom-notification-event'); - }; + initEventListeners() { + $(document).on('change', '.js-custom-notification-event', this.toggleCheckbox); + } - NotificationsForm.prototype.initEventListeners = function() { - return $(document).on('change', '.js-custom-notification-event', this.toggleCheckbox); - }; + toggleCheckbox(e) { + const $checkbox = $(e.currentTarget); + const $parent = $checkbox.closest('.checkbox'); - NotificationsForm.prototype.toggleCheckbox = function(e) { - var $checkbox, $parent; - $checkbox = $(e.currentTarget); - $parent = $checkbox.closest('.checkbox'); - return this.saveEvent($checkbox, $parent); - }; + this.saveEvent($checkbox, $parent); + } - NotificationsForm.prototype.showCheckboxLoadingSpinner = function($parent) { - return $parent.addClass('is-loading').find('.custom-notification-event-loading').removeClass('fa-check').addClass('fa-spin fa-spinner').removeClass('is-done'); - }; + // eslint-disable-next-line class-methods-use-this + showCheckboxLoadingSpinner($parent) { + $parent.addClass('is-loading') + .find('.custom-notification-event-loading') + .removeClass('fa-check') + .addClass('fa-spin fa-spinner') + .removeClass('is-done'); + } - NotificationsForm.prototype.saveEvent = function($checkbox, $parent) { - var form; - form = $parent.parents('form:first'); - return $.ajax({ - url: form.attr('action'), - method: form.attr('method'), - dataType: 'json', - data: form.serialize(), - beforeSend: (function(_this) { - return function() { - return _this.showCheckboxLoadingSpinner($parent); - }; - })(this) - }).done(function(data) { - $checkbox.enable(); - if (data.saved) { - $parent.find('.custom-notification-event-loading').toggleClass('fa-spin fa-spinner fa-check is-done'); - return setTimeout(function() { - return $parent.removeClass('is-loading').find('.custom-notification-event-loading').toggleClass('fa-spin fa-spinner fa-check is-done'); - }, 2000); - } - }); - }; + saveEvent($checkbox, $parent) { + const form = $parent.parents('form:first'); - return NotificationsForm; - })(); -}).call(window); + return $.ajax({ + url: form.attr('action'), + method: form.attr('method'), + dataType: 'json', + data: form.serialize(), + beforeSend: () => { + this.showCheckboxLoadingSpinner($parent); + }, + }).done((data) => { + $checkbox.enable(); + if (data.saved) { + $parent.find('.custom-notification-event-loading').toggleClass('fa-spin fa-spinner fa-check is-done'); + setTimeout(() => { + $parent.removeClass('is-loading') + .find('.custom-notification-event-loading') + .toggleClass('fa-spin fa-spinner fa-check is-done'); + }, 2000); + } + }); + } +} diff --git a/app/assets/javascripts/pager.js b/app/assets/javascripts/pager.js index 6792b984cc5..6552a88b606 100644 --- a/app/assets/javascripts/pager.js +++ b/app/assets/javascripts/pager.js @@ -1,78 +1,74 @@ import { getParameterByName } from '~/lib/utils/common_utils'; import { removeParams } from './lib/utils/url_utility'; -(() => { - const ENDLESS_SCROLL_BOTTOM_PX = 400; - const ENDLESS_SCROLL_FIRE_DELAY_MS = 1000; +const ENDLESS_SCROLL_BOTTOM_PX = 400; +const ENDLESS_SCROLL_FIRE_DELAY_MS = 1000; - const Pager = { - init(limit = 0, preload = false, disable = false, prepareData = $.noop, callback = $.noop) { - this.url = $('.content_list').data('href') || removeParams(['limit', 'offset']); - this.limit = limit; - this.offset = parseInt(getParameterByName('offset'), 10) || this.limit; - this.disable = disable; - this.prepareData = prepareData; - this.callback = callback; - this.loading = $('.loading').first(); - if (preload) { - this.offset = 0; - this.getOld(); - } - this.initLoadMore(); - }, +export default { + init(limit = 0, preload = false, disable = false, prepareData = $.noop, callback = $.noop) { + this.url = $('.content_list').data('href') || removeParams(['limit', 'offset']); + this.limit = limit; + this.offset = parseInt(getParameterByName('offset'), 10) || this.limit; + this.disable = disable; + this.prepareData = prepareData; + this.callback = callback; + this.loading = $('.loading').first(); + if (preload) { + this.offset = 0; + this.getOld(); + } + this.initLoadMore(); + }, - getOld() { - this.loading.show(); - $.ajax({ - type: 'GET', - url: this.url, - data: `limit=${this.limit}&offset=${this.offset}`, - dataType: 'json', - error: () => this.loading.hide(), - success: (data) => { - this.append(data.count, this.prepareData(data.html)); - this.callback(); + getOld() { + this.loading.show(); + $.ajax({ + type: 'GET', + url: this.url, + data: `limit=${this.limit}&offset=${this.offset}`, + dataType: 'json', + error: () => this.loading.hide(), + success: (data) => { + this.append(data.count, this.prepareData(data.html)); + this.callback(); - // keep loading until we've filled the viewport height - if (!this.disable && !this.isScrollable()) { - this.getOld(); - } else { - this.loading.hide(); - } - }, - }); - }, + // keep loading until we've filled the viewport height + if (!this.disable && !this.isScrollable()) { + this.getOld(); + } else { + this.loading.hide(); + } + }, + }); + }, - append(count, html) { - $('.content_list').append(html); - if (count > 0) { - this.offset += count; - } else { - this.disable = true; - } - }, + append(count, html) { + $('.content_list').append(html); + if (count > 0) { + this.offset += count; + } else { + this.disable = true; + } + }, - isScrollable() { - const $w = $(window); - return $(document).height() > $w.height() + $w.scrollTop() + ENDLESS_SCROLL_BOTTOM_PX; - }, + isScrollable() { + const $w = $(window); + return $(document).height() > $w.height() + $w.scrollTop() + ENDLESS_SCROLL_BOTTOM_PX; + }, - initLoadMore() { - $(document).unbind('scroll'); - $(document).endlessScroll({ - bottomPixels: ENDLESS_SCROLL_BOTTOM_PX, - fireDelay: ENDLESS_SCROLL_FIRE_DELAY_MS, - fireOnce: true, - ceaseFire: () => this.disable === true, - callback: () => { - if (!this.loading.is(':visible')) { - this.loading.show(); - this.getOld(); - } - }, - }); - }, - }; - - window.Pager = Pager; -})(); + initLoadMore() { + $(document).unbind('scroll'); + $(document).endlessScroll({ + bottomPixels: ENDLESS_SCROLL_BOTTOM_PX, + fireDelay: ENDLESS_SCROLL_FIRE_DELAY_MS, + fireOnce: true, + ceaseFire: () => this.disable === true, + callback: () => { + if (!this.loading.is(':visible')) { + this.loading.show(); + this.getOld(); + } + }, + }); + }, +}; diff --git a/app/assets/javascripts/project_variables.js b/app/assets/javascripts/project_variables.js deleted file mode 100644 index 567c311f119..00000000000 --- a/app/assets/javascripts/project_variables.js +++ /dev/null @@ -1,39 +0,0 @@ - -const HIDDEN_VALUE_TEXT = '******'; - -export default class ProjectVariables { - constructor() { - this.$revealBtn = $('.js-btn-toggle-reveal-values'); - this.$revealBtn.on('click', this.toggleRevealState.bind(this)); - } - - toggleRevealState(e) { - e.preventDefault(); - - const oldStatus = this.$revealBtn.attr('data-status'); - let newStatus = 'hidden'; - let newAction = 'Reveal Values'; - - if (oldStatus === 'hidden') { - newStatus = 'revealed'; - newAction = 'Hide Values'; - } - - this.$revealBtn.attr('data-status', newStatus); - - const $variables = $('.variable-value'); - - $variables.each((_, variable) => { - const $variable = $(variable); - let newText = HIDDEN_VALUE_TEXT; - - if (newStatus === 'revealed') { - newText = $variable.attr('data-value'); - } - - $variable.text(newText); - }); - - this.$revealBtn.text(newAction); - } -} diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.js index 05c4a28be88..43b2d238f65 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.js @@ -65,10 +65,12 @@ export default { <div class="mr-widget-body media"> <status-icon status="success" /> <div class="media-body"> - <h4> - Set by - <mr-widget-author :author="mr.setToMWPSBy" /> - to be merged automatically when the pipeline succeeds + <h4 class="flex-container-block"> + <span class="append-right-10"> + Set by + <mr-widget-author :author="mr.setToMWPSBy" /> + to be merged automatically when the pipeline succeeds + </span> <a v-if="mr.canCancelAutomaticMerge" @click.prevent="cancelAutomaticMerge" @@ -94,8 +96,13 @@ export default { <p v-if="mr.shouldRemoveSourceBranch"> The source branch will be removed </p> - <p v-else> - The source branch will not be removed + <p + v-else + class="flex-container-block" + > + <span class="append-right-10"> + The source branch will not be removed + </span> <a v-if="canRemoveSourceBranch" :disabled="isRemovingSourceBranch" diff --git a/app/assets/javascripts/vue_shared/components/toggle_button.vue b/app/assets/javascripts/vue_shared/components/toggle_button.vue index ddc9ddbc3a3..4277d9281a0 100644 --- a/app/assets/javascripts/vue_shared/components/toggle_button.vue +++ b/app/assets/javascripts/vue_shared/components/toggle_button.vue @@ -1,6 +1,13 @@ <script> + import { s__ } from '../../locale'; + import icon from './icon.vue'; import loadingIcon from './loading_icon.vue'; + const ICON_ON = 'status_success_borderless'; + const ICON_OFF = 'status_failed_borderless'; + const LABEL_ON = s__('ToggleButton|Toggle Status: ON'); + const LABEL_OFF = s__('ToggleButton|Toggle Status: OFF'); + export default { props: { name: { @@ -22,19 +29,10 @@ required: false, default: false, }, - enabledText: { - type: String, - required: false, - default: 'Enabled', - }, - disabledText: { - type: String, - required: false, - default: 'Disabled', - }, }, components: { + icon, loadingIcon, }, @@ -43,6 +41,15 @@ event: 'change', }, + computed: { + toggleIcon() { + return this.value ? ICON_ON : ICON_OFF; + }, + ariaLabel() { + return this.value ? LABEL_ON : LABEL_OFF; + }, + }, + methods: { toggleFeature() { if (!this.disabledInput) this.$emit('change', !this.value); @@ -60,10 +67,8 @@ /> <button type="button" - aria-label="Toggle" class="project-feature-toggle" - :data-enabled-text="enabledText" - :data-disabled-text="disabledText" + :aria-label="ariaLabel" :class="{ 'is-checked': value, 'is-disabled': disabledInput, @@ -72,6 +77,11 @@ @click="toggleFeature" > <loadingIcon class="loading-icon" /> + <span class="toggle-icon"> + <icon + css-classes="toggle-icon-svg" + :name="toggleIcon"/> + </span> </button> </label> </template> diff --git a/app/assets/stylesheets/framework/contextual-sidebar.scss b/app/assets/stylesheets/framework/contextual-sidebar.scss index 26a2db99e0a..8baf7ca23a4 100644 --- a/app/assets/stylesheets/framework/contextual-sidebar.scss +++ b/app/assets/stylesheets/framework/contextual-sidebar.scss @@ -320,13 +320,14 @@ transition: width $sidebar-transition-duration; position: fixed; bottom: 0; - padding: 16px; + padding: $gl-padding; background-color: $gray-light; border: 0; border-top: 2px solid $border-color; color: $gl-text-color-secondary; display: flex; align-items: center; + line-height: 1; svg { margin-right: 8px; diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 609f33582e1..1588036aeae 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -396,3 +396,8 @@ span.idiff { .file-fork-suggestion-note { margin-right: 1.5em; } + +.label-lfs { + color: $common-gray-light; + border: 1px solid $common-gray-light; +} diff --git a/app/assets/stylesheets/framework/toggle.scss b/app/assets/stylesheets/framework/toggle.scss index 71765da3908..0cd83df218f 100644 --- a/app/assets/stylesheets/framework/toggle.scss +++ b/app/assets/stylesheets/framework/toggle.scss @@ -27,7 +27,7 @@ border: 0; outline: 0; display: block; - width: 100px; + width: 50px; height: 24px; cursor: pointer; user-select: none; @@ -42,31 +42,31 @@ background: none; } - &::before { - color: $feature-toggle-text-color; - font-size: 12px; - line-height: 24px; - position: absolute; - top: 0; - left: 25px; - right: 5px; - text-align: center; - overflow: hidden; - text-overflow: ellipsis; - animation: animate-disabled .2s ease-in; - content: attr(data-disabled-text); - } - - &::after { + .toggle-icon { position: relative; display: block; - content: ""; - width: 22px; - height: 18px; left: 0; border-radius: 9px; background: $feature-toggle-color; transition: all .2s ease; + + &, + .toggle-icon-svg { + width: 18px; + height: 18px; + } + + .toggle-icon-svg { + fill: $feature-toggle-color-disabled; + } + + .toggle-status-checked { + display: none; + } + + .toggle-status-unchecked { + display: inline; + } } .loading-icon { @@ -77,11 +77,10 @@ top: 50%; left: 50%; transform: translate(-50%, -50%); - } &.is-loading { - &::before { + .toggle-icon { display: none; } @@ -100,15 +99,20 @@ &.is-checked { background: $feature-toggle-color-enabled; - &::before { - left: 5px; - right: 25px; - animation: animate-enabled .2s ease-in; - content: attr(data-enabled-text); - } + .toggle-icon { + left: calc(100% - 18px); - &::after { - left: calc(100% - 22px); + .toggle-icon-svg { + fill: $feature-toggle-color-enabled; + } + + .toggle-status-checked { + display: inline; + } + + .toggle-status-unchecked { + display: none; + } } } diff --git a/app/controllers/concerns/boards_responses.rb b/app/controllers/concerns/boards_responses.rb index 2c9c095a5d7..a145049dc7d 100644 --- a/app/controllers/concerns/boards_responses.rb +++ b/app/controllers/concerns/boards_responses.rb @@ -24,11 +24,11 @@ module BoardsResponses end def respond_with_boards - respond_with(@boards) + respond_with(@boards) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def respond_with_board - respond_with(@board) + respond_with(@board) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def respond_with(resource) diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 782f0be9c4a..6f4fdcdaa4f 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -1,6 +1,8 @@ module CreatesCommit extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize + # rubocop:disable Gitlab/ModuleWithInstanceVariables def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) if can?(current_user, :push_code, @project) @project_to_commit_into = @project @@ -45,6 +47,7 @@ module CreatesCommit end end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def authorize_edit_tree! return if can_collaborate_with_project? @@ -77,6 +80,7 @@ module CreatesCommit end end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def new_merge_request_path project_new_merge_request_path( @project_to_commit_into, @@ -88,20 +92,28 @@ module CreatesCommit } ) end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def existing_merge_request_path - project_merge_request_path(@project, @merge_request) + project_merge_request_path(@project, @merge_request) # rubocop:disable Gitlab/ModuleWithInstanceVariables end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def merge_request_exists? - return @merge_request if defined?(@merge_request) - - @merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened - .find_by(source_project_id: @project_to_commit_into, source_branch: @branch_name, target_branch: @start_branch) + strong_memoize(:merge_request) do + MergeRequestsFinder.new(current_user, project_id: @project.id) + .execute + .opened + .find_by( + source_project_id: @project_to_commit_into, + source_branch: @branch_name, + target_branch: @start_branch) + end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def different_project? - @project_to_commit_into != @project + @project_to_commit_into != @project # rubocop:disable Gitlab/ModuleWithInstanceVariables end def create_merge_request? @@ -109,6 +121,6 @@ module CreatesCommit # as the target branch in the same project, # we don't want to create a merge request. params[:create_merge_request].present? && - (different_project? || @start_branch != @branch_name) + (different_project? || @start_branch != @branch_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index 9d4f97aa443..b10147835f3 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -1,4 +1,5 @@ module GroupTree + # rubocop:disable Gitlab/ModuleWithInstanceVariables def render_group_tree(groups) @groups = if params[:filter].present? Gitlab::GroupHierarchy.new(groups.search(params[:filter])) @@ -20,5 +21,6 @@ module GroupTree render json: serializer.represent(@groups) end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 281756af57a..c3013884369 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -17,7 +17,7 @@ module IssuableActions end def update - @issuable = update_service.execute(issuable) + @issuable = update_service.execute(issuable) # rubocop:disable Gitlab/ModuleWithInstanceVariables respond_to do |format| format.html do @@ -81,7 +81,7 @@ module IssuableActions private def recaptcha_check_if_spammable(should_redirect = true, &block) - return yield unless @issuable.is_a? Spammable + return yield unless issuable.is_a? Spammable recaptcha_check_with_fallback(should_redirect, &block) end @@ -89,7 +89,7 @@ module IssuableActions def render_conflict_response respond_to do |format| format.html do - @conflict = true + @conflict = true # rubocop:disable Gitlab/ModuleWithInstanceVariables render :edit end @@ -104,7 +104,7 @@ module IssuableActions end def labels - @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute + @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute # rubocop:disable Gitlab/ModuleWithInstanceVariables end def authorize_destroy_issuable! @@ -114,7 +114,7 @@ module IssuableActions end def authorize_admin_issuable! - unless can?(current_user, :"admin_#{resource_name}", @project) + unless can?(current_user, :"admin_#{resource_name}", @project) # rubocop:disable Gitlab/ModuleWithInstanceVariables return access_denied! end end @@ -148,6 +148,7 @@ module IssuableActions @resource_name ||= controller_name.singularize end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def render_entity_json if @issuable.valid? render json: serializer.represent(@issuable) @@ -155,6 +156,7 @@ module IssuableActions render json: { errors: @issuable.errors.full_messages }, status: :unprocessable_entity end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def serializer raise NotImplementedError @@ -165,6 +167,6 @@ module IssuableActions end def parent - @project || @group + @project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index f3c9251225f..b25e753a5ad 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -2,6 +2,7 @@ module IssuableCollections extend ActiveSupport::Concern include SortingHelper include Gitlab::IssuableMetadata + include Gitlab::Utils::StrongMemoize included do helper_method :finder @@ -9,6 +10,7 @@ module IssuableCollections private + # rubocop:disable Gitlab/ModuleWithInstanceVariables def set_issuables_index @issuables = issuables_collection @issuables = @issuables.page(params[:page]) @@ -33,6 +35,7 @@ module IssuableCollections @users.push(author) if author end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def issuables_collection finder.execute.preload(preload_for_collection) @@ -41,7 +44,7 @@ module IssuableCollections def redirect_out_of_range(total_pages) return false if total_pages.zero? - out_of_range = @issuables.current_page > total_pages + out_of_range = @issuables.current_page > total_pages # rubocop:disable Gitlab/ModuleWithInstanceVariables if out_of_range redirect_to(url_for(params.merge(page: total_pages, only_path: true))) @@ -51,7 +54,7 @@ module IssuableCollections end def issuable_page_count - page_count_for_relation(@issuables, finder.row_count) + page_count_for_relation(@issuables, finder.row_count) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def page_count_for_relation(relation, row_count) @@ -66,6 +69,7 @@ module IssuableCollections finder_class.new(current_user, filter_params) end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def filter_params set_sort_order_from_cookie set_default_state @@ -90,6 +94,7 @@ module IssuableCollections @filter_params.permit(IssuableFinder::VALID_PARAMS) end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def set_default_state params[:state] = 'opened' if params[:state].blank? @@ -129,9 +134,9 @@ module IssuableCollections end def finder - return @finder if defined?(@finder) - - @finder = issuable_finder_for(@finder_type) + strong_memoize(:finder) do + issuable_finder_for(@finder_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables + end end def collection_type diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb index ad594903331..d4cccbe6442 100644 --- a/app/controllers/concerns/issues_action.rb +++ b/app/controllers/concerns/issues_action.rb @@ -2,6 +2,7 @@ module IssuesAction extend ActiveSupport::Concern include IssuableCollections + # rubocop:disable Gitlab/ModuleWithInstanceVariables def issues @finder_type = IssuesFinder @label = finder.labels.first @@ -17,4 +18,5 @@ module IssuesAction format.atom { render layout: 'xml.atom' } end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb index 8b569a01afd..4d44df3bba9 100644 --- a/app/controllers/concerns/merge_requests_action.rb +++ b/app/controllers/concerns/merge_requests_action.rb @@ -2,6 +2,7 @@ module MergeRequestsAction extend ActiveSupport::Concern include IssuableCollections + # rubocop:disable Gitlab/ModuleWithInstanceVariables def merge_requests @finder_type = MergeRequestsFinder @label = finder.labels.first @@ -10,6 +11,7 @@ module MergeRequestsAction @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type) end + # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb index 081f3336780..d92cf8b4894 100644 --- a/app/controllers/concerns/milestone_actions.rb +++ b/app/controllers/concerns/milestone_actions.rb @@ -6,7 +6,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_merge_requests_tab", { - merge_requests: @milestone.sorted_merge_requests, + merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Gitlab/ModuleWithInstanceVariables show_project_name: true }) end @@ -18,7 +18,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_participants_tab", { - users: @milestone.participants + users: @milestone.participants # rubocop:disable Gitlab/ModuleWithInstanceVariables }) end end @@ -29,7 +29,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_labels_tab", { - labels: @milestone.labels + labels: @milestone.labels # rubocop:disable Gitlab/ModuleWithInstanceVariables }) end end @@ -43,6 +43,7 @@ module MilestoneActions } end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def milestone_redirect_path if @project project_milestone_path(@project, @milestone) @@ -52,4 +53,5 @@ module MilestoneActions dashboard_milestone_path(@milestone.safe_title, title: @milestone.title) end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index be2e1b47feb..e82a5650935 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -1,5 +1,6 @@ module NotesActions include RendersNotes + include Gitlab::Utils::StrongMemoize extend ActiveSupport::Concern included do @@ -30,6 +31,7 @@ module NotesActions render json: notes_json end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def create create_params = note_params.merge( merge_request_diff_head_sha: params[:merge_request_diff_head_sha], @@ -47,7 +49,9 @@ module NotesActions format.html { redirect_back_or_default } end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables + # rubocop:disable Gitlab/ModuleWithInstanceVariables def update @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) @@ -60,6 +64,7 @@ module NotesActions format.html { redirect_back_or_default } end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def destroy if note.editable? @@ -138,7 +143,7 @@ module NotesActions end else template = "discussions/_diff_discussion" - @fresh_discussion = true + @fresh_discussion = true # rubocop:disable Gitlab/ModuleWithInstanceVariables locals = { discussions: [discussion], on_image: on_image } end @@ -191,7 +196,7 @@ module NotesActions end def noteable - @noteable ||= notes_finder.target || @note&.noteable + @noteable ||= notes_finder.target || @note&.noteable # rubocop:disable Gitlab/ModuleWithInstanceVariables end def require_noteable! @@ -211,20 +216,21 @@ module NotesActions end def note_project - return @note_project if defined?(@note_project) - return nil unless project + strong_memoize(:note_project) do + return nil unless project - note_project_id = params[:note_project_id] + note_project_id = params[:note_project_id] - @note_project = - if note_project_id.present? - Project.find(note_project_id) - else - project - end + the_project = + if note_project_id.present? + Project.find(note_project_id) + else + project + end - return access_denied! unless can?(current_user, :create_note, @note_project) + return access_denied! unless can?(current_user, :create_note, the_project) - @note_project + the_project + end end end diff --git a/app/controllers/concerns/oauth_applications.rb b/app/controllers/concerns/oauth_applications.rb index 9849aa93fa6..f0a68f23566 100644 --- a/app/controllers/concerns/oauth_applications.rb +++ b/app/controllers/concerns/oauth_applications.rb @@ -14,6 +14,6 @@ module OauthApplications end def load_scopes - @scopes = Doorkeeper.configuration.scopes + @scopes ||= Doorkeeper.configuration.scopes end end diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb index e9b9e9b38bc..90bb7a87b45 100644 --- a/app/controllers/concerns/preview_markdown.rb +++ b/app/controllers/concerns/preview_markdown.rb @@ -1,6 +1,7 @@ module PreviewMarkdown extend ActiveSupport::Concern + # rubocop:disable Gitlab/ModuleWithInstanceVariables def preview_markdown result = PreviewMarkdownService.new(@project, current_user, params).execute @@ -20,4 +21,5 @@ module PreviewMarkdown } } end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/renders_commits.rb b/app/controllers/concerns/renders_commits.rb index bb2c1dfa00a..fb41dc1e8a8 100644 --- a/app/controllers/concerns/renders_commits.rb +++ b/app/controllers/concerns/renders_commits.rb @@ -1,6 +1,6 @@ module RendersCommits def prepare_commits_for_rendering(commits) - Banzai::CommitRenderer.render(commits, @project, current_user) + Banzai::CommitRenderer.render(commits, @project, current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables commits end diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb index 824ad06465c..e7ef297879f 100644 --- a/app/controllers/concerns/renders_notes.rb +++ b/app/controllers/concerns/renders_notes.rb @@ -1,4 +1,5 @@ module RendersNotes + # rubocop:disable Gitlab/ModuleWithInstanceVariables def prepare_notes_for_rendering(notes, noteable = nil) preload_noteable_for_regular_notes(notes) preload_max_access_for_authors(notes, @project) @@ -7,6 +8,7 @@ module RendersNotes notes end + # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index be2e6c7f193..3d61458c064 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -66,7 +66,7 @@ module ServiceParams FILTER_BLANK_PARAMS = [:password].freeze def service_params - dynamic_params = @service.event_channel_names + @service.event_names + dynamic_params = @service.event_channel_names + @service.event_names # rubocop:disable Gitlab/ModuleWithInstanceVariables service_params = params.permit(:id, service: ALLOWED_PARAMS_CE + dynamic_params) if service_params[:service].is_a?(Hash) diff --git a/app/controllers/concerns/snippets_actions.rb b/app/controllers/concerns/snippets_actions.rb index ffea712a833..9095cc7f783 100644 --- a/app/controllers/concerns/snippets_actions.rb +++ b/app/controllers/concerns/snippets_actions.rb @@ -4,6 +4,7 @@ module SnippetsActions def edit end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def raw disposition = params[:inline] == 'false' ? 'attachment' : 'inline' @@ -14,6 +15,7 @@ module SnippetsActions filename: @snippet.sanitized_file_name ) end + # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb index 03d8e188093..922aa58a00f 100644 --- a/app/controllers/concerns/spammable_actions.rb +++ b/app/controllers/concerns/spammable_actions.rb @@ -2,6 +2,7 @@ module SpammableActions extend ActiveSupport::Concern include Recaptcha::Verify + include Gitlab::Utils::StrongMemoize included do before_action :authorize_submit_spammable!, only: :mark_as_spam @@ -18,9 +19,9 @@ module SpammableActions private def ensure_spam_config_loaded! - return @spam_config_loaded if defined?(@spam_config_loaded) - - @spam_config_loaded = Gitlab::Recaptcha.load_configurations! + strong_memoize(:spam_config_loaded) do + Gitlab::Recaptcha.load_configurations! + end end def recaptcha_check_with_fallback(should_redirect = true, &fallback) diff --git a/app/controllers/concerns/toggle_subscription_action.rb b/app/controllers/concerns/toggle_subscription_action.rb index 92cb534343e..776583579e8 100644 --- a/app/controllers/concerns/toggle_subscription_action.rb +++ b/app/controllers/concerns/toggle_subscription_action.rb @@ -12,7 +12,7 @@ module ToggleSubscriptionAction private def subscribable_project - @project || raise(NotImplementedError) + @project ||= raise(NotImplementedError) end def subscribable_resource diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 1c4c09c772f..4865ec3dfe5 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -110,7 +110,7 @@ class Projects::JobsController < Projects::ApplicationController def erase if @build.erase(erased_by: current_user) redirect_to project_job_path(project, @build), - notice: "Build has been successfully erased!" + notice: "Job has been successfully erased!" else respond_422 end diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 627cb2bd93c..5940fae8dd0 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -11,7 +11,7 @@ class Projects::NotesController < Projects::ApplicationController # Controller actions are returned from AbstractController::Base and methods of parent classes are # excluded in order to return only specific controller related methods. # That is ok for the app (no :create method in ancestors) - # but fails for tests because there is a :create method on FactoryGirl (one of the ancestors) + # but fails for tests because there is a :create method on FactoryBot (one of the ancestors) # # see https://github.com/rails/rails/blob/v4.2.7/actionpack/lib/abstract_controller/base.rb#L78 # diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb index f3719059f88..f752a46f828 100644 --- a/app/controllers/projects/tree_controller.rb +++ b/app/controllers/projects/tree_controller.rb @@ -26,6 +26,7 @@ class Projects::TreeController < Projects::ApplicationController respond_to do |format| format.html do + lfs_blob_ids @last_commit = @repository.last_commit_for_path(@commit.id, @tree.path) || @commit end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 8e9d6766d80..6f609348402 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -9,6 +9,7 @@ class ProjectsController < Projects::ApplicationController before_action :repository, except: [:index, :new, :create] before_action :assign_ref_vars, only: [:show], if: :repo_exists? before_action :tree, only: [:show], if: [:repo_exists?, :project_view_files?] + before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?] before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export] # Authorize diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index b43eaeaeea0..c013e5a708f 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -44,13 +44,11 @@ module Mentionable end def all_references(current_user = nil, extractor: nil) - @extractors ||= {} - # Use custom extractor if it's passed in the function parameters. if extractor - @extractors[current_user] = extractor + extractors[current_user] = extractor else - extractor = @extractors[current_user] ||= Gitlab::ReferenceExtractor.new(project, current_user) + extractor = extractors[current_user] ||= Gitlab::ReferenceExtractor.new(project, current_user) extractor.reset_memoized_values end @@ -69,6 +67,10 @@ module Mentionable extractor end + def extractors + @extractors ||= {} + end + def mentioned_users(current_user = nil) all_references(current_user).users end diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index 7026f565706..fd6703831e4 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -103,9 +103,11 @@ module Milestoneish end def memoize_per_user(user, method_name) - @memoized ||= {} - @memoized[method_name] ||= {} - @memoized[method_name][user&.id] ||= yield + memoized_users[method_name][user&.id] ||= yield + end + + def memoized_users + @memoized_users ||= Hash.new { |h, k| h[k] = {} } end # override in a class that includes this module to get a faster query diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index 5d75b2aa6a3..86f28f30032 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -46,6 +46,7 @@ module Noteable notes.inc_relations_for_view.grouped_diff_discussions(*args) end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def resolvable_discussions @resolvable_discussions ||= if defined?(@discussions) @@ -54,6 +55,7 @@ module Noteable discussion_notes.resolvable.discussions(self) end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def discussions_resolvable? resolvable_discussions.any?(&:resolvable?) diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb index ce69fd34ac5..e48bc0be410 100644 --- a/app/models/concerns/participable.rb +++ b/app/models/concerns/participable.rb @@ -56,15 +56,17 @@ module Participable # # Returns an Array of User instances. def participants(current_user = nil) - @participants ||= Hash.new do |hash, user| - hash[user] = raw_participants(user) - end - - @participants[current_user] + all_participants[current_user] end private + def all_participants + @all_participants ||= Hash.new do |hash, user| + hash[user] = raw_participants(user) + end + end + def raw_participants(current_user = nil) current_user ||= author ext = Gitlab::ReferenceExtractor.new(project, current_user) diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index e961c97e337..835f26aa57b 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -52,7 +52,7 @@ module RelativePositioning # to its predecessor. This process will recursively move all the predecessors until we have a place if (after.relative_position - before.relative_position) < 2 before.move_before - @positionable_neighbours = [before] + @positionable_neighbours = [before] # rubocop:disable Gitlab/ModuleWithInstanceVariables end self.relative_position = position_between(before.relative_position, after.relative_position) @@ -65,7 +65,7 @@ module RelativePositioning if before.shift_after? issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_after) issue_to_move.move_after - @positionable_neighbours = [issue_to_move] + @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables pos_after = issue_to_move.relative_position end @@ -80,7 +80,7 @@ module RelativePositioning if after.shift_before? issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_before) issue_to_move.move_before - @positionable_neighbours = [issue_to_move] + @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables pos_before = issue_to_move.relative_position end @@ -132,6 +132,7 @@ module RelativePositioning end end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def save_positionable_neighbours return unless @positionable_neighbours @@ -140,4 +141,5 @@ module RelativePositioning status end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb index f006a271327..b6c7b6735b9 100644 --- a/app/models/concerns/resolvable_discussion.rb +++ b/app/models/concerns/resolvable_discussion.rb @@ -31,15 +31,11 @@ module ResolvableDiscussion end def resolvable? - return @resolvable if @resolvable.present? - - @resolvable = potentially_resolvable? && notes.any?(&:resolvable?) + @resolvable ||= potentially_resolvable? && notes.any?(&:resolvable?) end def resolved? - return @resolved if @resolved.present? - - @resolved = resolvable? && notes.none?(&:to_be_resolved?) + @resolved ||= resolvable? && notes.none?(&:to_be_resolved?) end def first_note @@ -49,13 +45,13 @@ module ResolvableDiscussion def first_note_to_resolve return unless resolvable? - @first_note_to_resolve ||= notes.find(&:to_be_resolved?) + @first_note_to_resolve ||= notes.find(&:to_be_resolved?) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def last_resolved_note return unless resolved? - @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last + @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last # rubocop:disable Gitlab/ModuleWithInstanceVariables end def resolved_notes @@ -95,7 +91,7 @@ module ResolvableDiscussion yield(notes_relation) # Set the notes array to the updated notes - @notes = notes_relation.fresh.to_a + @notes = notes_relation.fresh.to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables self.class.memoized_values.each do |var| instance_variable_set(:"@#{var}", nil) diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index 22fde2eb134..5c1cce98ad4 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -88,7 +88,7 @@ module Routable def full_name if route && route.name.present? - @full_name ||= route.name + @full_name ||= route.name # rubocop:disable Gitlab/ModuleWithInstanceVariables else update_route if persisted? @@ -112,7 +112,7 @@ module Routable def expires_full_path_cache RequestStore.delete(full_path_key) if RequestStore.active? - @full_path = nil + @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end def build_full_path @@ -127,7 +127,7 @@ module Routable def uncached_full_path if route && route.path.present? - @full_path ||= route.path + @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables else update_route if persisted? @@ -166,7 +166,7 @@ module Routable route || build_route(source: self) route.path = build_full_path route.name = build_full_name - @full_path = nil - @full_name = nil + @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables + @full_name = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb index 731d9b9a745..5e4274619c4 100644 --- a/app/models/concerns/spammable.rb +++ b/app/models/concerns/spammable.rb @@ -12,6 +12,7 @@ module Spammable attr_accessor :spam attr_accessor :spam_log + alias_method :spam?, :spam after_validation :check_for_spam, on: [:create, :update] @@ -34,10 +35,6 @@ module Spammable end end - def spam? - @spam - end - def check_for_spam error_msg = if Gitlab::Recaptcha.enabled? "Your #{spammable_entity_type} has been recognized as spam. "\ diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index 25e2d8ea24e..d07041c2fdf 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -39,7 +39,7 @@ module Taskable def task_list_items return [] if description.blank? - @task_list_items ||= Taskable.get_tasks(description) + @task_list_items ||= Taskable.get_tasks(description) # rubocop:disable Gitlab/ModuleWithInstanceVariables end def tasks diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index 9f403d96ed5..89fe6527647 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -21,6 +21,7 @@ module TimeTrackable has_many :timelogs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def spend_time(options) @time_spent = options[:duration] @time_spent_user = options[:user] @@ -36,6 +37,7 @@ module TimeTrackable end end alias_method :spend_time=, :spend_time + # rubocop:enable Gitlab/ModuleWithInstanceVariables def total_time_spent timelogs.sum(:time_spent) @@ -52,9 +54,10 @@ module TimeTrackable private def reset_spent_time - timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) + timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def add_or_subtract_spent_time timelogs.new( time_spent: time_spent, @@ -62,16 +65,19 @@ module TimeTrackable spent_at: @spent_at ) end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def check_negative_time_spent return if time_spent.nil? || time_spent == :reset - # we need to cache the total time spent so multiple calls to #valid? - # doesn't give a false error - @original_total_time_spent ||= total_time_spent - - if time_spent < 0 && (time_spent.abs > @original_total_time_spent) + if time_spent < 0 && (time_spent.abs > original_total_time_spent) errors.add(:time_spent, 'Time to subtract exceeds the total time spent') end end + + # we need to cache the total time spent so multiple calls to #valid? + # doesn't give a false error + def original_total_time_spent + @original_total_time_spent ||= total_time_spent + end end diff --git a/app/models/identity.rb b/app/models/identity.rb index ff811e19f8a..99d99bc6deb 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -14,11 +14,11 @@ class Identity < ActiveRecord::Base end def ldap? - provider.starts_with?('ldap') + Gitlab::OAuth::Provider.ldap_provider?(provider) end def self.normalize_uid(provider, uid) - if provider.to_s.starts_with?('ldap') + if Gitlab::OAuth::Provider.ldap_provider?(provider) Gitlab::LDAP::Person.normalize_dn(uid) else uid.to_s diff --git a/app/models/user.rb b/app/models/user.rb index 92b461ce3ed..51941f43919 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -738,7 +738,7 @@ class User < ActiveRecord::Base def ldap_user? if identities.loaded? - identities.find { |identity| identity.provider.start_with?('ldap') && !identity.extern_uid.nil? } + identities.find { |identity| Gitlab::OAuth::Provider.ldap_provider?(identity.provider) && !identity.extern_uid.nil? } else identities.exists?(["provider LIKE ? AND extern_uid IS NOT NULL", "ldap%"]) end diff --git a/app/models/user_synced_attributes_metadata.rb b/app/models/user_synced_attributes_metadata.rb index 9f374304164..548b99b69d9 100644 --- a/app/models/user_synced_attributes_metadata.rb +++ b/app/models/user_synced_attributes_metadata.rb @@ -6,11 +6,11 @@ class UserSyncedAttributesMetadata < ActiveRecord::Base SYNCABLE_ATTRIBUTES = %i[name email location].freeze def read_only?(attribute) - Gitlab.config.omniauth.sync_profile_from_provider && synced?(attribute) + sync_profile_from_provider? && synced?(attribute) end def read_only_attributes - return [] unless Gitlab.config.omniauth.sync_profile_from_provider + return [] unless sync_profile_from_provider? SYNCABLE_ATTRIBUTES.select { |key| synced?(key) } end @@ -22,4 +22,10 @@ class UserSyncedAttributesMetadata < ActiveRecord::Base def set_attribute_synced(attribute, value) write_attribute("#{attribute}_synced", value) end + + private + + def sync_profile_from_provider? + Gitlab::OAuth::Provider.sync_profile_from_provider?(provider) + end end diff --git a/app/serializers/concerns/with_pagination.rb b/app/serializers/concerns/with_pagination.rb index d29e22d6740..89631b73fcf 100644 --- a/app/serializers/concerns/with_pagination.rb +++ b/app/serializers/concerns/with_pagination.rb @@ -14,7 +14,7 @@ module WithPagination # we shouldn't try to paginate single resources def represent(resource, opts = {}) if paginated? && resource.respond_to?(:page) - super(@paginator.paginate(resource), opts) + super(paginator.paginate(resource), opts) else super(resource, opts) end diff --git a/app/services/concerns/issues/resolve_discussions.rb b/app/services/concerns/issues/resolve_discussions.rb index 7d45b4aa26a..26eb274f4d5 100644 --- a/app/services/concerns/issues/resolve_discussions.rb +++ b/app/services/concerns/issues/resolve_discussions.rb @@ -1,24 +1,28 @@ module Issues module ResolveDiscussions + include Gitlab::Utils::StrongMemoize + attr_reader :merge_request_to_resolve_discussions_of_iid, :discussion_to_resolve_id + # rubocop:disable Gitlab/ModuleWithInstanceVariables def filter_resolve_discussion_params @merge_request_to_resolve_discussions_of_iid ||= params.delete(:merge_request_to_resolve_discussions_of) @discussion_to_resolve_id ||= params.delete(:discussion_to_resolve) end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def merge_request_to_resolve_discussions_of - return @merge_request_to_resolve_discussions_of if defined?(@merge_request_to_resolve_discussions_of) - - @merge_request_to_resolve_discussions_of = MergeRequestsFinder.new(current_user, project_id: project.id) - .execute - .find_by(iid: merge_request_to_resolve_discussions_of_iid) + strong_memoize(:merge_request_to_resolve_discussions_of) do + MergeRequestsFinder.new(current_user, project_id: project.id) + .execute + .find_by(iid: merge_request_to_resolve_discussions_of_iid) + end end def discussions_to_resolve return [] unless merge_request_to_resolve_discussions_of - @discussions_to_resolve ||= + @discussions_to_resolve ||= # rubocop:disable Gitlab/ModuleWithInstanceVariables if discussion_to_resolve_id discussion_or_nil = merge_request_to_resolve_discussions_of .find_discussion(discussion_to_resolve_id) diff --git a/app/services/spam_check_service.rb b/app/services/spam_check_service.rb index 11030bee8f1..d4ade869777 100644 --- a/app/services/spam_check_service.rb +++ b/app/services/spam_check_service.rb @@ -7,16 +7,19 @@ # - params with :request # module SpamCheckService + # rubocop:disable Gitlab/ModuleWithInstanceVariables def filter_spam_check_params @request = params.delete(:request) @api = params.delete(:api) @recaptcha_verified = params.delete(:recaptcha_verified) @spam_log_id = params.delete(:spam_log_id) end + # rubocop:enable Gitlab/ModuleWithInstanceVariables # In order to be proceed to the spam check process, @spammable has to be # a dirty instance, which means it should be already assigned with the new # attribute values. + # rubocop:disable Gitlab/ModuleWithInstanceVariables def spam_check(spammable, user) spam_service = SpamService.new(spammable, @request) @@ -24,4 +27,5 @@ module SpamCheckService user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/views/admin/system_info/show.html.haml b/app/views/admin/system_info/show.html.haml index 6bf979a937e..23f9927cfee 100644 --- a/app/views/admin/system_info/show.html.haml +++ b/app/views/admin/system_info/show.html.haml @@ -15,7 +15,7 @@ Unable to collect CPU info .col-sm-4 .light-well - %h4 Memory + %h4 Memory Usage .data - if @memory %h1 #{number_to_human_size(@memory.active_bytes)} / #{number_to_human_size(@memory.total_bytes)} @@ -24,7 +24,7 @@ Unable to collect memory info .col-sm-4 .light-well - %h4 Disks + %h4 Disk Usage .data - @disks.each do |disk| %h1 #{number_to_human_size(disk[:bytes_used])} / #{number_to_human_size(disk[:bytes_total])} @@ -34,4 +34,4 @@ .light-well %h4 Uptime .data - %h1= time_ago_with_tooltip(Rails.application.config.booted_at) + %h1= distance_of_time_in_words_to_now(Rails.application.config.booted_at) diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 98ff592eb64..63c5a15de1c 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -157,7 +157,6 @@ %ul %li User will not be able to login %li User will not be able to access git repositories - %li User will be removed from joined projects and groups %li Personal projects will be left %li Owned groups will be left %br diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index 2bac69bc536..6e399fc7392 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -10,5 +10,7 @@ %p.settings-message.text-center.append-bottom-0 No variables found, add one with the form above. - else - = render "ci/variables/table" - %button.btn.btn-info.js-btn-toggle-reveal-values{ "data-status" => 'hidden' } Reveal Values + .js-secret-variable-table + = render "ci/variables/table" + %button.btn.btn-info.js-secret-value-reveal-button{ data: { secret_reveal_status: 'false' } } + = n_('Reveal value', 'Reveal values', @variables.size) diff --git a/app/views/ci/variables/_table.html.haml b/app/views/ci/variables/_table.html.haml index 71a0b56c4f4..2298930d0c7 100644 --- a/app/views/ci/variables/_table.html.haml +++ b/app/views/ci/variables/_table.html.haml @@ -15,7 +15,11 @@ - if variable.id? %tr %td.variable-key= variable.key - %td.variable-value{ "data-value" => variable.value }****** + %td.variable-value + %span.js-secret-value-placeholder + = '*' * 6 + %span.hide.js-secret-value + = variable.value %td.variable-protected= Gitlab::Utils.boolean_to_yes_no(variable.protected) %td.variable-menu = link_to variable.edit_path, class: "btn btn-transparent btn-variable-edit" do diff --git a/app/views/projects/_merge_request_merge_settings.html.haml b/app/views/projects/_merge_request_merge_settings.html.haml index 1dd8778f800..f6e5712ce81 100644 --- a/app/views/projects/_merge_request_merge_settings.html.haml +++ b/app/views/projects/_merge_request_merge_settings.html.haml @@ -8,7 +8,7 @@ %br %span.descr Pipelines need to be configured to enable this feature. - = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds') + = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds'), target: '_blank' .checkbox = form.label :only_allow_merge_if_all_discussions_are_resolved do = form.check_box :only_allow_merge_if_all_discussions_are_resolved diff --git a/app/views/projects/blob/_header_content.html.haml b/app/views/projects/blob/_header_content.html.haml index 98bedae650a..5d457a50c49 100644 --- a/app/views/projects/blob/_header_content.html.haml +++ b/app/views/projects/blob/_header_content.html.haml @@ -8,3 +8,6 @@ %small = number_to_human_size(blob.raw_size) + + - if blob.stored_externally? && blob.external_storage == :lfs + %span.label.label-lfs.append-right-5 LFS diff --git a/app/views/projects/clusters/_cluster.html.haml b/app/views/projects/clusters/_cluster.html.haml index 18ca01d2d49..ad696daa259 100644 --- a/app/views/projects/clusters/_cluster.html.haml +++ b/app/views/projects/clusters/_cluster.html.haml @@ -16,7 +16,8 @@ class: "js-toggle-cluster-list project-feature-toggle #{'is-checked' if cluster.enabled?} #{'is-disabled' if !cluster.can_toggle_cluster?}", "aria-label": s_("ClusterIntegration|Toggle Cluster"), disabled: !cluster.can_toggle_cluster?, - data: { "enabled-text": s_("ClusterIntegration|Active"), - "disabled-text": s_("ClusterIntegration|Inactive"), - endpoint: namespace_project_cluster_path(@project.namespace, @project, cluster, format: :json) } } + data: { endpoint: namespace_project_cluster_path(@project.namespace, @project, cluster, format: :json) } } = icon("spinner spin", class: "loading-icon") + %span.toggle-icon + = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked') + = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked') diff --git a/app/views/projects/clusters/_enabled.html.haml b/app/views/projects/clusters/_enabled.html.haml index 70c677f7856..547b3c8446f 100644 --- a/app/views/projects/clusters/_enabled.html.haml +++ b/app/views/projects/clusters/_enabled.html.haml @@ -7,8 +7,10 @@ %button{ type: 'button', class: "js-toggle-cluster project-feature-toggle #{'is-checked' unless !@cluster.enabled?} #{'is-disabled' unless can?(current_user, :update_cluster, @cluster)}", "aria-label": s_("ClusterIntegration|Toggle Cluster"), - disabled: !can?(current_user, :update_cluster, @cluster), - data: { "enabled-text": s_("ClusterIntegration|Active"), "disabled-text": s_("ClusterIntegration|Inactive"), } } + disabled: !can?(current_user, :update_cluster, @cluster) } + %span.toggle-icon + = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked') + = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked') - if can?(current_user, :update_cluster, @cluster) .form-group diff --git a/app/views/projects/pipelines_settings/_show.html.haml b/app/views/projects/pipelines_settings/_show.html.haml index c63e716180c..c5f9f5aa15b 100644 --- a/app/views/projects/pipelines_settings/_show.html.haml +++ b/app/views/projects/pipelines_settings/_show.html.haml @@ -40,10 +40,14 @@ = form.text_field :domain, class: 'form-control', placeholder: 'domain.com' %hr - .form-group.append-bottom-default + .form-group.append-bottom-default.js-secret-runner-token = f.label :runners_token, "Runner token", class: 'label-light' - = f.text_field :runners_token, class: "form-control", placeholder: 'xEeFCaDAB89' + .form-control.js-secret-value-placeholder + = '*' * 20 + = f.text_field :runners_token, class: "form-control hide js-secret-value", placeholder: 'xEeFCaDAB89' %p.help-block The secure token used by the Runner to checkout the project + %button.btn.btn-info.prepend-top-10.js-secret-value-reveal-button{ type: 'button', data: { secret_reveal_status: 'false' } } + = _('Reveal value') %hr .form-group diff --git a/app/views/projects/tree/_blob_item.html.haml b/app/views/projects/tree/_blob_item.html.haml index c51af901699..8c1c532cb3e 100644 --- a/app/views/projects/tree/_blob_item.html.haml +++ b/app/views/projects/tree/_blob_item.html.haml @@ -1,9 +1,12 @@ +- is_lfs_blob = @lfs_blob_ids.include?(blob_item.id) %tr{ class: "tree-item #{tree_hex_class(blob_item)}" } %td.tree-item-file-name = tree_icon(type, blob_item.mode, blob_item.name) - file_name = blob_item.name = link_to project_blob_path(@project, tree_join(@id || @commit.id, blob_item.name)), class: 'str-truncated', title: file_name do %span= file_name + - if is_lfs_blob + %span.label.label-lfs.prepend-left-5 LFS %td.hidden-xs.tree-commit %td.tree-time-ago.cgray.text-right = render 'projects/tree/spinner' diff --git a/app/workers/concerns/new_issuable.rb b/app/workers/concerns/new_issuable.rb index eb0d6c9c36c..526ed0bad07 100644 --- a/app/workers/concerns/new_issuable.rb +++ b/app/workers/concerns/new_issuable.rb @@ -9,15 +9,15 @@ module NewIssuable end def set_user(user_id) - @user = User.find_by(id: user_id) + @user = User.find_by(id: user_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables - log_error(User, user_id) unless @user + log_error(User, user_id) unless @user # rubocop:disable Gitlab/ModuleWithInstanceVariables end def set_issuable(issuable_id) - @issuable = issuable_class.find_by(id: issuable_id) + @issuable = issuable_class.find_by(id: issuable_id) # rubocop:disable Gitlab/ModuleWithInstanceVariables - log_error(issuable_class, issuable_id) unless @issuable + log_error(issuable_class, issuable_id) unless @issuable # rubocop:disable Gitlab/ModuleWithInstanceVariables end def log_error(record_class, record_id) diff --git a/changelogs/unreleased/38019-hide-runner-token.yml b/changelogs/unreleased/38019-hide-runner-token.yml new file mode 100644 index 00000000000..11ae0a685ef --- /dev/null +++ b/changelogs/unreleased/38019-hide-runner-token.yml @@ -0,0 +1,5 @@ +--- +title: Hide runner token in CI/CD settings page +merge_request: +author: +type: added diff --git a/changelogs/unreleased/38145_ux_issues_in_system_info_page.yml b/changelogs/unreleased/38145_ux_issues_in_system_info_page.yml new file mode 100644 index 00000000000..d2358750518 --- /dev/null +++ b/changelogs/unreleased/38145_ux_issues_in_system_info_page.yml @@ -0,0 +1,5 @@ +--- +title: Fixes the wording of headers in system info page +merge_request: 15802 +author: Gilbert Roulot +type: fixed diff --git a/changelogs/unreleased/38239-update-toggle-design.yml b/changelogs/unreleased/38239-update-toggle-design.yml new file mode 100644 index 00000000000..4d9034e8515 --- /dev/null +++ b/changelogs/unreleased/38239-update-toggle-design.yml @@ -0,0 +1,5 @@ +--- +title: Update feature toggle design to use icons and make it i18n friendly +merge_request: 15904 +author: +type: changed diff --git a/changelogs/unreleased/38541-cancel-alignment.yml b/changelogs/unreleased/38541-cancel-alignment.yml new file mode 100644 index 00000000000..c6d5136dd57 --- /dev/null +++ b/changelogs/unreleased/38541-cancel-alignment.yml @@ -0,0 +1,5 @@ +--- +title: fix button alignment on MWPS component +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/40285-prometheus-loading-screen-no-longer-seems-to-appear.yml b/changelogs/unreleased/40285-prometheus-loading-screen-no-longer-seems-to-appear.yml deleted file mode 100644 index 978930a5b8c..00000000000 --- a/changelogs/unreleased/40285-prometheus-loading-screen-no-longer-seems-to-appear.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix broken illustration images for monitoring page empty states -merge_request: 15889 -author: -type: fixed diff --git a/changelogs/unreleased/40555-replace-absolute-urls-with-related-branches-to-avoid-hostname.yml b/changelogs/unreleased/40555-replace-absolute-urls-with-related-branches-to-avoid-hostname.yml deleted file mode 100644 index 4f0eaf8472f..00000000000 --- a/changelogs/unreleased/40555-replace-absolute-urls-with-related-branches-to-avoid-hostname.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Fix related branches/Merge requests failing to load when the hostname setting - is changed -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/40711-fix-forking-hashed-projects.yml b/changelogs/unreleased/40711-fix-forking-hashed-projects.yml deleted file mode 100644 index 116d7d4e9cf..00000000000 --- a/changelogs/unreleased/40711-fix-forking-hashed-projects.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix the fork project functionality for projects with hashed storage -merge_request: 15671 -author: -type: fixed diff --git a/changelogs/unreleased/40715-updateendpoint-undefined-on-issue-page.yml b/changelogs/unreleased/40715-updateendpoint-undefined-on-issue-page.yml deleted file mode 100644 index 0328a693354..00000000000 --- a/changelogs/unreleased/40715-updateendpoint-undefined-on-issue-page.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix updateEndpoint undefined error for issue_show app root -merge_request: 15698 -author: -type: fixed diff --git a/changelogs/unreleased/41016-import-gitlab-shell-projects.yml b/changelogs/unreleased/41016-import-gitlab-shell-projects.yml new file mode 100644 index 00000000000..47a9e9c3eec --- /dev/null +++ b/changelogs/unreleased/41016-import-gitlab-shell-projects.yml @@ -0,0 +1,6 @@ +--- +title: Import some code and functionality from gitlab-shell to improve subprocess + handling +merge_request: +author: +type: other diff --git a/changelogs/unreleased/bvl-circuitbreaker-keys-set.yml b/changelogs/unreleased/bvl-circuitbreaker-keys-set.yml deleted file mode 100644 index a56456240df..00000000000 --- a/changelogs/unreleased/bvl-circuitbreaker-keys-set.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Keep track of all circuitbreaker keys in a set -merge_request: 15613 -author: -type: performance diff --git a/changelogs/unreleased/bvl-double-fork.yml b/changelogs/unreleased/bvl-double-fork.yml deleted file mode 100644 index 50bc1adde26..00000000000 --- a/changelogs/unreleased/bvl-double-fork.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Correctly link to a forked project from the new fork page. -merge_request: 15653 -author: -type: fixed diff --git a/changelogs/unreleased/bvl-fork-networks-for-deleted-projects.yml b/changelogs/unreleased/bvl-fork-networks-for-deleted-projects.yml deleted file mode 100644 index 2acb98db785..00000000000 --- a/changelogs/unreleased/bvl-fork-networks-for-deleted-projects.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Create a fork network for forks with a deleted source -merge_request: 15595 -author: -type: fixed diff --git a/changelogs/unreleased/dm-ldap-email-readonly.yml b/changelogs/unreleased/dm-ldap-email-readonly.yml new file mode 100644 index 00000000000..744b21f901e --- /dev/null +++ b/changelogs/unreleased/dm-ldap-email-readonly.yml @@ -0,0 +1,5 @@ +--- +title: Make sure user email is read only when synced with LDAP +merge_request: 15915 +author: +type: fixed diff --git a/changelogs/unreleased/lfs-badge.yml b/changelogs/unreleased/lfs-badge.yml new file mode 100644 index 00000000000..e4ed4d6741f --- /dev/null +++ b/changelogs/unreleased/lfs-badge.yml @@ -0,0 +1,5 @@ +--- +title: Added badge to tree & blob views to indicate LFS tracked files +merge_request: +author: +type: added diff --git a/changelogs/unreleased/protected-branches-names.yml b/changelogs/unreleased/protected-branches-names.yml deleted file mode 100644 index 3c6767df571..00000000000 --- a/changelogs/unreleased/protected-branches-names.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Only load branch names for protected branch checks -merge_request: -author: -type: performance diff --git a/changelogs/unreleased/remove-incorrect-guidance.yml b/changelogs/unreleased/remove-incorrect-guidance.yml new file mode 100644 index 00000000000..eeb5745698f --- /dev/null +++ b/changelogs/unreleased/remove-incorrect-guidance.yml @@ -0,0 +1,6 @@ +--- +title: Removed incorrect guidance stating blocked users will be removed from groups + and project as members +merge_request: 15947 +author: CesarApodaca +type: fixed diff --git a/changelogs/unreleased/sh-fix-import-rake-task.yml b/changelogs/unreleased/sh-fix-import-rake-task.yml deleted file mode 100644 index 9cd6d7e4a72..00000000000 --- a/changelogs/unreleased/sh-fix-import-rake-task.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix gitlab:import:repos Rake task moving repositories into the wrong location -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/sh-fix-root-ref-repository.yml b/changelogs/unreleased/sh-fix-root-ref-repository.yml deleted file mode 100644 index 0670db84fa6..00000000000 --- a/changelogs/unreleased/sh-fix-root-ref-repository.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: "Gracefully handle case when repository's root ref does not exist" -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/sh-optimize-groups-api.yml b/changelogs/unreleased/sh-optimize-groups-api.yml deleted file mode 100644 index 37b74715a81..00000000000 --- a/changelogs/unreleased/sh-optimize-groups-api.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Optimize API /groups/:id/projects by preloading associations -merge_request: -author: -type: performance diff --git a/config/application.rb b/config/application.rb index 6436f887d14..1110199b888 100644 --- a/config/application.rb +++ b/config/application.rb @@ -34,6 +34,10 @@ module Gitlab config.generators.templates.push("#{config.root}/generator_templates") + # Rake tasks ignore the eager loading settings, so we need to set the + # autoload paths explicitly + config.autoload_paths = config.eager_load_paths.dup + # Only load the plugins named here, in the order given (default is alphabetical). # :all can be used as a placeholder for all plugins not explicitly named. # config.plugins = [ :exception_notification, :ssl_requirement, :all ] @@ -159,7 +163,7 @@ module Gitlab config.middleware.insert_after ActionDispatch::Flash, 'Gitlab::Middleware::ReadOnly' config.generators do |g| - g.factory_girl false + g.factory_bot false end config.after_initialize do diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index c8b6018bc1b..f2f05b3eeb2 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -383,6 +383,7 @@ production: &base # Sync user's profile from the specified Omniauth providers every time the user logs in (default: empty). # Define the allowed providers using an array, e.g. ["cas3", "saml", "twitter"], # or as true/false to allow all providers or none. + # When authenticating using LDAP, the user's email is always synced. # sync_profile_from_provider: [] # Select which info to sync from the providers above. (default: email). diff --git a/config/initializers/fix_local_cache_middleware.rb b/config/initializers/fix_local_cache_middleware.rb index cb37f9ed22c..2644ee6a7d3 100644 --- a/config/initializers/fix_local_cache_middleware.rb +++ b/config/initializers/fix_local_cache_middleware.rb @@ -6,7 +6,7 @@ module LocalCacheRegistryCleanupWithEnsure def call(env) LocalCacheRegistry.set_cache_for(local_cache_key, LocalStore.new) - response = @app.call(env) + response = @app.call(env) # rubocop:disable Gitlab/ModuleWithInstanceVariables response[2] = ::Rack::BodyProxy.new(response[2]) do LocalCacheRegistry.set_cache_for(local_cache_key, nil) end diff --git a/config/initializers/rspec_profiling.rb b/config/initializers/rspec_profiling.rb index 16b9d5b15e5..2de310753a9 100644 --- a/config/initializers/rspec_profiling.rb +++ b/config/initializers/rspec_profiling.rb @@ -19,10 +19,10 @@ module RspecProfilingExt def example_finished(*args) super rescue => err - return if @already_logged_example_finished_error + return if @already_logged_example_finished_error # rubocop:disable Gitlab/ModuleWithInstanceVariables $stderr.puts "rspec_profiling couldn't collect an example: #{err}. Further warnings suppressed." - @already_logged_example_finished_error = true + @already_logged_example_finished_error = true # rubocop:disable Gitlab/ModuleWithInstanceVariables end alias_method :example_passed, :example_finished diff --git a/config/initializers/rugged_use_gitlab_git_attributes.rb b/config/initializers/rugged_use_gitlab_git_attributes.rb index 7d652799786..1cfb3bcb4bd 100644 --- a/config/initializers/rugged_use_gitlab_git_attributes.rb +++ b/config/initializers/rugged_use_gitlab_git_attributes.rb @@ -15,8 +15,11 @@ module Rugged class Repository module UseGitlabGitAttributes def fetch_attributes(name, *) + attributes.attributes(name) + end + + def attributes @attributes ||= Gitlab::Git::Attributes.new(path) - @attributes.attributes(name) end end diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index 96c6d954ff7..d7be6f5950f 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -140,8 +140,8 @@ class Gitlab::Seeder::CycleAnalytics issue.update(milestone: @project.milestones.sample) else label_name = "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" - list_label = FactoryGirl.create(:label, title: label_name, project: issue.project) - FactoryGirl.create(:list, board: FactoryGirl.create(:board, project: issue.project), label: list_label) + list_label = FactoryBot.create(:label, title: label_name, project: issue.project) + FactoryBot.create(:list, board: FactoryBot.create(:board, project: issue.project), label: list_label) issue.update(labels: [list_label]) end diff --git a/doc/api/groups.md b/doc/api/groups.md index c1b5737c247..de730cdd869 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -365,13 +365,15 @@ POST /groups Parameters: -- `name` (required) - The name of the group -- `path` (required) - The path of the group -- `description` (optional) - The group's description -- `visibility` (optional) - The group's visibility. Can be `private`, `internal`, or `public`. -- `lfs_enabled` (optional) - Enable/disable Large File Storage (LFS) for the projects in this group -- `request_access_enabled` (optional) - Allow users to request member access. -- `parent_id` (optional) - The parent group id for creating nested group. +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `name` | string | yes | The name of the group | +| `path` | string | yes | The path of the group | +| `description` | string | no | The group's description | +| `visibility` | string | no | The group's visibility. Can be `private`, `internal`, or `public`. | +| `lfs_enabled` | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group | +| `request_access_enabled` | boolean | no | Allow users to request member access. | +| `parent_id` | integer | no | The parent group id for creating nested group. | ## Transfer project to group @@ -383,8 +385,10 @@ POST /groups/:id/projects/:project_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user -- `project_id` (required) - The ID or path of a project +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user | +| `project_id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | ## Update group diff --git a/doc/development/README.md b/doc/development/README.md index 7e4c767692a..b624aa37c70 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -37,6 +37,7 @@ comments: false - [`Gemfile` guidelines](gemfile.md) - [Sidekiq debugging](sidekiq_debugging.md) - [Gotchas](gotchas.md) to avoid +- [Avoid modules with instance variables](module_with_instance_variables.md) if possible - [Issue and merge requests state models](object_state_models.md) - [How to dump production data to staging](db_dump.md) - [Working with the GitHub importer](github_importer.md) @@ -81,10 +82,9 @@ comments: false ## Documentation guides -- [Documentation styleguide](doc_styleguide.md): Use this styleguide if you are - contributing to the documentation. - [Writing documentation](writing_documentation.md) - - [Distinction between general documentation and technical articles](writing_documentation.md#distinction-between-general-documentation-and-technical-articles) +- [Documentation styleguide](doc_styleguide.md) +- [Markdown](../user/markdown.md) ## Internationalization (i18n) guides diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md index c2ca8966a3f..5786287d00c 100644 --- a/doc/development/gotchas.md +++ b/doc/development/gotchas.md @@ -8,7 +8,7 @@ might encounter or should avoid during development of GitLab CE and EE. Consider the following factory: ```ruby -FactoryGirl.define do +FactoryBot.define do factory :label do sequence(:title) { |n| "label#{n}" } end @@ -53,7 +53,7 @@ When run, this spec doesn't do what we might expect: (compared using ==) ``` -That's because FactoryGirl sequences are not reseted for each example. +That's because FactoryBot sequences are not reseted for each example. Please remember that sequence-generated values exist only to avoid having to explicitly set attributes that have a uniqueness constraint when using a factory. diff --git a/doc/development/i18n/index.md b/doc/development/i18n/index.md index 4cb2624c098..8aa0462d213 100644 --- a/doc/development/i18n/index.md +++ b/doc/development/i18n/index.md @@ -59,6 +59,7 @@ Requests to become a proof reader will be considered on the merits of previous t - French - German - Italian + - [Paolo Falomo](https://crowdin.com/profile/paolo.falomo) - Japanese - Korean - [Huang Tao](https://crowdin.com/profile/htve) diff --git a/doc/development/module_with_instance_variables.md b/doc/development/module_with_instance_variables.md new file mode 100644 index 00000000000..48a1b7f847e --- /dev/null +++ b/doc/development/module_with_instance_variables.md @@ -0,0 +1,242 @@ +## Modules with instance variables could be considered harmful + +### Background + +Rails somehow encourages people using modules and instance variables +everywhere. For example, using instance variables in the controllers, +helpers, and views. They're also encouraging the use of +`ActiveSupport::Concern`, which further strengthens the idea of +saving everything in a giant, single object, and people could access +everything in that one giant object. + +### The problems + +Of course this is convenient to develop, because we just have everything +within reach. However this has a number of downsides when that chosen object +is growing, it would later become out of control for the same reason. + +There are just too many things in the same context, and we don't know if +those things are tightly coupled or not, depending on each others or not. +It's very hard to tell when the complexity grows to a point, and it makes +tracking the code also extremely hard. For example, a class could be using +3 different instance variables, and all of them could be initialized and +manipulated from 3 different modules. It's hard to track when those variables +start giving us troubles. We don't know which module would suddenly change +one of the variables. Everything could touch anything. + +### Similar concerns + +People are saying multiple inheritance is bad. Mixing multiple modules with +multiple instance variables scattering everywhere suffer from the same issue. +The same applies to `ActiveSupport::Concern`. See: +[Consider replacing concerns with dedicated classes & composition]( +https://gitlab.com/gitlab-org/gitlab-ce/issues/23786) + +There's also a similar idea: +[Use decorators and interface segregation to solve overgrowing models problem]( +https://gitlab.com/gitlab-org/gitlab-ce/issues/13484) + +Note that `included` doesn't solve the whole issue. They define the +dependencies, but they still allow each modules to talk implicitly via the +instance variables in the final giant object, and that's where the problem is. + +### Solutions + +We should split the giant object into multiple objects, and they communicate +with each other with the API, i.e. public methods. In short, composition over +inheritance. This way, each smaller objects would have their own respective +limited states, i.e. instance variables. If one instance variable goes wrong, +we would be very clear that it's from that single small object, because +no one else could be touching it. + +With clearly defined API, this would make things less coupled and much easier +to debug and track, and much more extensible for other objects to use, because +they communicate in a clear way, rather than implicit dependencies. + +### Acceptable use + +However, it's not always bad to use instance variables in a module, +as long as it's contained in the same module; that is, no other modules or +objects are touching them, then it would be an acceptable use. + +We especially allow the case where a single instance variable is used with +`||=` to setup the value. This would look like: + +``` ruby +module M + def f + @f ||= true + end +end +``` + +Unfortunately it's not easy to code more complex rules into the cop, so +we rely on people's best judgement. If we could find another good pattern +we could easily add to the cop, we should do it. + +### How to rewrite and avoid disabling this cop + +Even if we could just disable the cop, we should avoid doing so. Some code +could be easily rewritten in simple form. Consider this acceptable method: + +``` ruby +module Gitlab + module Emoji + def emoji_unicode_version(name) + @emoji_unicode_versions_by_name ||= + JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json'))) + @emoji_unicode_versions_by_name[name] + end + end +end +``` + +This method is totally fine because it's already self-contained. No other +methods should be using `@emoji_unicode_versions_by_name` and we're good. +However it's still offending the cop because it's not just `||=`, and the +cop is not smart enough to judge that this is fine. + +On the other hand, we could split this method into two: + +``` ruby +module Gitlab + module Emoji + def emoji_unicode_version(name) + emoji_unicode_versions_by_name[name] + end + + private + + def emoji_unicode_versions_by_name + @emoji_unicode_versions_by_name ||= + JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json'))) + end + end +end +``` + +Now the cop won't complain. Here's a bad example which we could rewrite: + +``` ruby +module SpamCheckService + def filter_spam_check_params + @request = params.delete(:request) + @api = params.delete(:api) + @recaptcha_verified = params.delete(:recaptcha_verified) + @spam_log_id = params.delete(:spam_log_id) + end + + def spam_check(spammable, user) + spam_service = SpamService.new(spammable, @request) + + spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do + user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) + end + end +end +``` + +There are several implicit dependencies here. First, `params` should be +defined before use. Second, `filter_spam_check_params` should be called +before `spam_check`. These are all implicit and the includer could be using +those instance variables without awareness. + +This should be rewritten like: + +``` ruby +class SpamCheckService + def initialize(request:, api:, recaptcha_verified:, spam_log_id:) + @request = request + @api = api + @recaptcha_verified = recaptcha_verified + @spam_log_id = spam_log_id + end + + def spam_check(spammable, user) + spam_service = SpamService.new(spammable, @request) + + spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do + user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) + end + end +end +``` + +And use it like: + +``` ruby +class UpdateSnippetService < BaseService + def execute + # ... + spam = SpamCheckService.new(params.slice!(:request, :api, :recaptcha_verified, :spam_log_id)) + + spam.check(snippet, current_user) + # ... + end +end +``` + +This way, all those instance variables are isolated in `SpamCheckService` +rather than whatever includes the module, and those modules which were also +included, making it much easier to track down any issues, +and reducing the chance of having name conflicts. + +### How to disable this cop + +Put the disabling comment right after your code in the same line: + +``` ruby +module M + def violating_method + @f + @g # rubocop:disable Gitlab/ModuleWithInstanceVariables + end +end +``` + +If there are multiple lines, you could also enable and disable for a section: + +``` ruby +module M + # rubocop:disable Gitlab/ModuleWithInstanceVariables + def violating_method + @f = 0 + @g = 1 + @h = 2 + end + # rubocop:enable Gitlab/ModuleWithInstanceVariables +end +``` + +Note that you need to enable it at some point, otherwise everything below +won't be checked. + +### Things we might need to ignore right now + +Because of the way Rails helpers and mailers work, we might not be able to +avoid the use of instance variables there. For those cases, we could ignore +them at the moment. At least we're not going to share those modules with +other random objects, so they're still somewhat isolated. + +### Instance variables in views + +They're bad because we can't easily tell who's using the instance variables +(from controller's point of view) and where we set them up (from partials' +point of view), making it extremely hard to track data dependency. + +We're trying to use something like this instead: + +``` haml += render 'projects/commits/commit', commit: commit, ref: ref, project: project +``` + +And in the partial: + +``` haml +- ref = local_assigns.fetch(:ref) +- commit = local_assigns.fetch(:commit) +- project = local_assigns.fetch(:project) +``` + +This way it's clearer where those values were coming from, and we gain the +benefit to have typo check over using instance variables. In the future, +we should also forbid the use of instance variables in partials. diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 8b7b015427f..edb8f372ea3 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -8,8 +8,8 @@ and effective _as well as_ fast. Here are some things to keep in mind regarding test performance: -- `double` and `spy` are faster than `FactoryGirl.build(...)` -- `FactoryGirl.build(...)` and `.build_stubbed` are faster than `.create`. +- `double` and `spy` are faster than `FactoryBot.build(...)` +- `FactoryBot.build(...)` and `.build_stubbed` are faster than `.create`. - Don't `create` an object when `build`, `build_stubbed`, `attributes_for`, `spy`, or `double` will do. Database persistence is slow! - Don't mark a feature as requiring JavaScript (through `@javascript` in @@ -254,13 +254,13 @@ end ### Factories -GitLab uses [factory_girl] as a test fixture replacement. +GitLab uses [factory_bot] as a test fixture replacement. - Factory definitions live in `spec/factories/`, named using the pluralization of their corresponding model (`User` factories are defined in `users.rb`). - There should be only one top-level factory definition per file. -- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and - should) call `create(...)` instead of `FactoryGirl.create(...)`. +- FactoryBot methods are mixed in to all RSpec groups. This means you can (and + should) call `create(...)` instead of `FactoryBot.create(...)`. - Make use of [traits] to clean up definitions and usages. - When defining a factory, don't define attributes that are not required for the resulting record to pass validation. @@ -269,8 +269,8 @@ GitLab uses [factory_girl] as a test fixture replacement. - Factories don't have to be limited to `ActiveRecord` objects. [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). -[factory_girl]: https://github.com/thoughtbot/factory_girl -[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits +[factory_bot]: https://github.com/thoughtbot/factory_bot +[traits]: http://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#Traits ### Fixtures diff --git a/doc/development/testing_guide/index.md b/doc/development/testing_guide/index.md index 8045bbad7ba..65386f231a0 100644 --- a/doc/development/testing_guide/index.md +++ b/doc/development/testing_guide/index.md @@ -33,7 +33,7 @@ changes should be tested. ## [Testing best practices](best_practices.md) -Everything you should know about how to write good tests: RSpec, FactoryGirl, +Everything you should know about how to write good tests: RSpec, FactoryBot, system tests, parameterized tests etc. --- diff --git a/doc/development/writing_documentation.md b/doc/development/writing_documentation.md index 48e04a40050..43a79ffcaa5 100644 --- a/doc/development/writing_documentation.md +++ b/doc/development/writing_documentation.md @@ -1,75 +1,14 @@ # Writing documentation - **General Documentation**: written by the [developers responsible by creating features](#contributing-to-docs). Should be submitted in the same merge request containing code. Feature proposals (by GitLab contributors) should also be accompanied by its respective documentation. They can be later improved by PMs and Technical Writers. - - **Technical Articles**: written by any [GitLab Team](https://about.gitlab.com/team/) member, GitLab contributors, or [Community Writers](https://about.gitlab.com/handbook/product/technical-writing/community-writers/). + - **[Technical Articles](#technical-articles)**: written by any [GitLab Team](https://about.gitlab.com/team/) member, GitLab contributors, or [Community Writers](https://about.gitlab.com/handbook/product/technical-writing/community-writers/). - **Indexes per topic**: initially prepared by the Technical Writing Team, and kept up-to-date by developers and PMs in the same merge request containing code. They gather all resources for that topic in a single page (user and admin documentation, articles, and third-party docs). -## Distinction between General Documentation and Technical Articles - -### General documentation - -General documentation is categorized by _User_, _Admin_, and _Contributor_, and describe what that feature is, what it does, and its available settings. - -### Technical Articles - -Technical articles replace technical content that once lived in the [GitLab Blog](https://about.gitlab.com/blog/), where they got out-of-date and weren't easily found. - -They are topic-related documentation, written with an user-friendly approach and language, aiming to provide the community with guidance on specific processes to achieve certain objectives. - -A technical article guides users and/or admins to achieve certain objectives (within guides and tutorials), or provide an overview of that particular topic or feature (within technical overviews). It can also describe the use, implementation, or integration of third-party tools with GitLab. - -They live under `doc/articles/article-title/index.md`, and their images should be placed under `doc/articles/article-title/img/`. Find a list of existing [technical articles](../articles/index.md) here. - -#### Types of Technical Articles - -- **User guides**: technical content to guide regular users from point A to point B -- **Admin guides**: technical content to guide administrators of GitLab instances from point A to point B -- **Technical Overviews**: technical content describing features, solutions, and third-party integrations -- **Tutorials**: technical content provided step-by-step on how to do things, or how to reach very specific objectives - -#### Understanding guides, tutorials, and technical overviews - -Suppose there's a process to go from point A to point B in 5 steps: `(A) 1 > 2 > 3 > 4 > 5 (B)`. - -A **guide** can be understood as a description of certain processes to achieve a particular objective. A guide brings you from A to B describing the characteristics of that process, but not necessarily going over each step. It can mention, for example, steps 2 and 3, but does not necessarily explain how to accomplish them. - -- Live example: "GitLab Pages from A to Z - [Part 1](../user/project/pages/getting_started_part_one.md) to [Part 4](../user/project/pages/getting_started_part_four.md)" - -A **tutorial** requires a clear **step-by-step** guidance to achieve a singular objective. It brings you from A to B, describing precisely all the necessary steps involved in that process, showing each of the 5 steps to go from A to B. -It does not only describes steps 2 and 3, but also shows you how to accomplish them. - -- Live example (on the blog): [Hosting on GitLab.com with GitLab Pages](https://about.gitlab.com/2016/04/07/gitlab-pages-setup/) - -A **technical overview** is a description of what a certain feature is, and what it does, but does not walk -through the process of how to use it systematically. - -- Live example (on the blog): [GitLab Workflow, an overview](https://about.gitlab.com/2016/10/25/gitlab-workflow-an-overview/) - -#### Special format - -Every **Technical Article** contains, in the very beginning, a blockquote with the following information: - -- A reference to the **type of article** (user guide, admin guide, tech overview, tutorial) -- A reference to the **knowledge level** expected from the reader to be able to follow through (beginner, intermediate, advanced) -- A reference to the **author's name** and **GitLab.com handle** -- A reference of the **publication date** - -```md -> **Article [Type](../../development/writing_documentation.html#types-of-technical-articles):** tutorial || -> **Level:** intermediary || -> **Author:** [Name Surname](https://gitlab.com/username) || -> **Publication date:** AAAA/MM/DD -``` - -#### Technical Articles - Writing Method - -Use the [writing method](https://about.gitlab.com/handbook/product/technical-writing/#writing-method) defined by the Technical Writing team. - ## Documentation style guidelines All the docs follow the same [styleguide](doc_styleguide.md). -### Contributing to docs +## Contributing to docs Whenever a feature is changed, updated, introduced, or deprecated, the merge request introducing these changes must be accompanied by the documentation @@ -118,13 +57,31 @@ and for every **major** feature present in Community Edition. Currently GitLab docs use Redcarpet as [markdown](../user/markdown.md) engine, but there's an [open discussion](https://gitlab.com/gitlab-com/gitlab-docs/issues/50) for implementing Kramdown in the near future. -## Testing +### Previewing locally -We try to treat documentation as code, thus have implemented some testing. +To preview your changes to documentation locally, please follow +this [development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development). + +### Testing + +We treat documentation as code, thus have implemented some testing. Currently, the following tests are in place: 1. `docs lint`: Check that all internal (relative) links work correctly and - that all cURL examples in API docs use the full switches. + that all cURL examples in API docs use the full switches. It's recommended + to [check locally](#previewing-locally) before pushing to GitLab by executing the command + `bundle exec nanoc check internal_links` on your local + [`gitlab-docs`](https://gitlab.com/gitlab-com/gitlab-docs) directory. +1. [`ee_compat_check`](https://docs.gitlab.com/ee/development/automatic_ce_ee_merge.html#avoiding-ce-gt-ee-merge-conflicts-beforehand) (runs on CE only): + When you submit a merge request to GitLab Community Edition (CE), + there is this additional job that runs against Enterprise Edition (EE) + and checks if your changes can apply cleanly to the EE codebase. + If that job fails, read the instructions in the job log for what to do next. + As CE is merged into EE once a day, it's important to avoid merge conflicts. + Submitting an EE-equivalent merge request cherry-picking all commits from CE to EE is + essential to avoid them. + +### Branch naming If your contribution contains **only** documentation changes, you can speed up the CI process by following some branch naming conventions. You have three @@ -139,17 +96,7 @@ choices: If your branch name matches any of the above, it will run only the docs tests. If it doesn't, the whole test suite will run (including docs). ---- - -When you submit a merge request to GitLab Community Edition (CE), there is an -additional job called `ee_compat_check` that runs against Enterprise -Edition (EE) and checks if your changes can apply cleanly to the EE codebase. -If that job fails, read the instructions in the job log for what to do next. -Contributors do not need to submit their changes to EE, GitLab Inc. employees -on the other hand need to make sure that their changes apply cleanly to both -CE and EE. - -## Previewing the changes live +### Previewing the changes live If you want to preview the doc changes of your merge request live, you can use the manual `review-docs-deploy` job in your merge request. You will need at @@ -164,7 +111,7 @@ You will need to push a branch to those repositories, it doesn't work for forks. TIP: **Tip:** If your branch contains only documentation changes, you can use -[special branch names](#testing) to avoid long running pipelines. +[special branch names](#branch-naming) to avoid long running pipelines. In the mini pipeline graph, you should see an `>>` icon. Clicking on it will reveal the `review-docs-deploy` job. Hit the play button for the job to start. @@ -209,12 +156,12 @@ working on. If you don't, the remote docs branch won't be removed either, and the server where the Review Apps are hosted will eventually be out of disk space. -### Behind the scenes +#### Technical aspects If you want to know the hot details, here's what's really happening: 1. You manually run the `review-docs-deploy` job in a CE/EE merge request. -1. The job runs the [`scirpts/trigger-build-docs`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/trigger-build-docs) +1. The job runs the [`scripts/trigger-build-docs`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/trigger-build-docs) script with the `deploy` flag, which in turn: 1. Takes your branch name and applies the following: - The slug of the branch name is used to avoid special characters since @@ -243,3 +190,65 @@ The following GitLab features are used among others: - [Review Apps](../ci/review_apps/index.md) - [Artifacts](../ci/yaml/README.md#artifacts) - [Specific Runner](../ci/runners/README.md#locking-a-specific-runner-from-being-enabled-for-other-projects) + +## General Documentation vs Technical Articles + +### General documentation + +General documentation is categorized by _User_, _Admin_, and _Contributor_, and describe what that feature is, what it does, and its available settings. + +### Technical Articles + +Technical articles replace technical content that once lived in the [GitLab Blog](https://about.gitlab.com/blog/), where they got out-of-date and weren't easily found. + +They are topic-related documentation, written with an user-friendly approach and language, aiming to provide the community with guidance on specific processes to achieve certain objectives. + +A technical article guides users and/or admins to achieve certain objectives (within guides and tutorials), or provide an overview of that particular topic or feature (within technical overviews). It can also describe the use, implementation, or integration of third-party tools with GitLab. + +They should be placed in a new directory named `/article-title/index.md` under a topic-related folder, and their images should be placed in `/article-title/img/`. For example, a new article on GitLab Pages should be placed in `doc/user/project/pages/article-title/` and a new article on GitLab CI/CD should be placed in `doc/ci/article-title/`. + +#### Types of Technical Articles + +- **User guides**: technical content to guide regular users from point A to point B +- **Admin guides**: technical content to guide administrators of GitLab instances from point A to point B +- **Technical Overviews**: technical content describing features, solutions, and third-party integrations +- **Tutorials**: technical content provided step-by-step on how to do things, or how to reach very specific objectives + +#### Understanding guides, tutorials, and technical overviews + +Suppose there's a process to go from point A to point B in 5 steps: `(A) 1 > 2 > 3 > 4 > 5 (B)`. + +A **guide** can be understood as a description of certain processes to achieve a particular objective. A guide brings you from A to B describing the characteristics of that process, but not necessarily going over each step. It can mention, for example, steps 2 and 3, but does not necessarily explain how to accomplish them. + +- Live example: "GitLab Pages from A to Z - [Part 1](../user/project/pages/getting_started_part_one.md) to [Part 4](../user/project/pages/getting_started_part_four.md)" + +A **tutorial** requires a clear **step-by-step** guidance to achieve a singular objective. It brings you from A to B, describing precisely all the necessary steps involved in that process, showing each of the 5 steps to go from A to B. +It does not only describes steps 2 and 3, but also shows you how to accomplish them. + +- Live example (on the blog): [Hosting on GitLab.com with GitLab Pages](https://about.gitlab.com/2016/04/07/gitlab-pages-setup/) + +A **technical overview** is a description of what a certain feature is, and what it does, but does not walk +through the process of how to use it systematically. + +- Live example (on the blog): [GitLab Workflow, an overview](https://about.gitlab.com/2016/10/25/gitlab-workflow-an-overview/) + +#### Special format + +Every **Technical Article** contains, in the very beginning, a blockquote with the following information: + +- A reference to the **type of article** (user guide, admin guide, tech overview, tutorial) +- A reference to the **knowledge level** expected from the reader to be able to follow through (beginner, intermediate, advanced) +- A reference to the **author's name** and **GitLab.com handle** +- A reference of the **publication date** + +```md +> **[Article Type](../../development/writing_documentation.html#types-of-technical-articles):** tutorial || +> **Level:** intermediary || +> **Author:** [Name Surname](https://gitlab.com/username) || +> **Publication date:** AAAA-MM-DD +``` + +#### Technical Articles - Writing Method + +Use the [writing method](https://about.gitlab.com/handbook/product/technical-writing/#writing-method) defined by the Technical Writing team. + diff --git a/doc/install/installation.md b/doc/install/installation.md index 6c6e5db4cd9..56888b05609 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -80,7 +80,7 @@ Make sure you have the right version of Git installed # Install Git sudo apt-get install -y git-core - # Make sure Git is version 2.13.6 or higher + # Make sure Git is version 2.14.3 or higher git --version Is the system packaged Git too old? Remove it and compile from source. diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 0e20b8096e9..20087a981f9 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -229,16 +229,18 @@ In order to enable/disable an OmniAuth provider, go to Admin Area -> Settings -> ## Keep OmniAuth user profiles up to date You can enable profile syncing from selected OmniAuth providers and for all or for specific user information. - + +When authenticating using LDAP, the user's email is always synced. + ```ruby gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2'] gitlab_rails['sync_profile_attributes'] = ['name', 'email', 'location'] ``` - + **For installations from source** - + ```yaml omniauth: sync_profile_from_provider: ['twitter', 'google_oauth2'] - sync_profile_claims_from_provider: ['email', 'location'] - ```
\ No newline at end of file + sync_profile_attributes: ['email', 'location'] + ``` diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index d100b431721..0b48596006d 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -19,6 +19,7 @@ project in an easy and automatic way: 1. [Auto Build](#auto-build) 1. [Auto Test](#auto-test) 1. [Auto Code Quality](#auto-code-quality) +1. [Auto SAST (Static Application Security Testing)](#auto-sast) 1. [Auto Review Apps](#auto-review-apps) 1. [Auto Deploy](#auto-deploy) 1. [Auto Monitoring](#auto-monitoring) @@ -147,6 +148,10 @@ has a `.gitlab-ci.yml` or not: do that in a branch to test Auto DevOps before committing to `master`. NOTE: **Note:** +Starting with GitLab 10.3, when enabling Auto DevOps, a pipeline is +automatically run on the default branch. + +NOTE: **Note:** If you are a GitLab Administrator, you can enable Auto DevOps instance wide in **Admin Area > Settings > Continuous Integration and Deployment**. Doing that, all the projects that haven't explicitly set an option will have Auto DevOps @@ -198,6 +203,18 @@ out. In GitLab Enterprise Edition Starter, differences between the source and target branches are [shown in the merge request widget](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html). +### Auto SAST + +> Introduced in [GitLab Enterprise Edition Ultimate][ee] 10.3. + +Static Application Security Testing (SAST) uses the +[gl-sast Docker image](https://gitlab.com/gitlab-org/gl-sast) to run static +analysis on the current code and checks for potential security issues. Once the +report is created, it's uploaded as an artifact which you can later download and +check out. + +Any security warnings are also [shown in the merge request widget](https://docs.gitlab.com/ee/user/project/merge_requests/sast.html). + ### Auto Review Apps NOTE: **Note:** @@ -536,3 +553,4 @@ curl --data "value=true" --header "PRIVATE-TOKEN: personal_access_token" https:/ [postgresql]: https://www.postgresql.org/ [Auto DevOps template]: https://gitlab.com/gitlab-org/gitlab-ci-yml/blob/master/Auto-DevOps.gitlab-ci.yml [GitLab Omnibus Helm Chart]: ../../install/kubernetes/gitlab_omnibus.md +[ee]: https://about.gitlab.com/gitlab-ee/ diff --git a/features/support/env.rb b/features/support/env.rb index 5962745d501..91a92314959 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -27,7 +27,7 @@ Spinach.hooks.before_run do # web editor and merge TestEnv.disable_pre_receive - include FactoryGirl::Syntax::Methods + include FactoryBot::Syntax::Methods include GitlabRoutingHelper end @@ -42,11 +42,11 @@ module StdoutReporterWithScenarioLocation # Override the standard reporter to show filename and line number next to each # scenario for easy, focused re-runs def before_scenario_run(scenario, step_definitions = nil) - @max_step_name_length = scenario.steps.map(&:name).map(&:length).max if scenario.steps.any? + @max_step_name_length = scenario.steps.map(&:name).map(&:length).max if scenario.steps.any? # rubocop:disable Gitlab/ModuleWithInstanceVariables name = scenario.name # This number has no significance, it's just to line things up - max_length = @max_step_name_length + 19 + max_length = @max_step_name_length + 19 # rubocop:disable Gitlab/ModuleWithInstanceVariables out.puts "\n #{'Scenario:'.green} #{name.light_green.ljust(max_length)}" \ " # #{scenario.feature.filename}:#{scenario.line}" end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 686bf7a3c2b..9ba15893f55 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -32,6 +32,11 @@ module API end end + # rubocop:disable Gitlab/ModuleWithInstanceVariables + # We can't rewrite this with StrongMemoize because `sudo!` would + # actually write to `@current_user`, and `sudo?` would immediately + # call `current_user` again which reads from `@current_user`. + # We should rewrite this in a way that using StrongMemoize is possible def current_user return @current_user if defined?(@current_user) @@ -45,6 +50,7 @@ module API @current_user end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def sudo? initial_current_user != current_user @@ -415,6 +421,7 @@ module API private + # rubocop:disable Gitlab/ModuleWithInstanceVariables def initial_current_user return @initial_current_user if defined?(@initial_current_user) @@ -424,6 +431,7 @@ module API unauthorized! end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def sudo! return unless sudo_identifier @@ -443,7 +451,7 @@ module API sudoed_user = find_user(sudo_identifier) not_found!("User with ID or username '#{sudo_identifier}'") unless sudoed_user - @current_user = sudoed_user + @current_user = sudoed_user # rubocop:disable Gitlab/ModuleWithInstanceVariables end def sudo_identifier diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index d6dea4c30e3..eff1c5b70ea 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -6,18 +6,16 @@ module API 'git-upload-pack' => [:ssh_upload_pack, Gitlab::GitalyClient::MigrationStatus::OPT_OUT] }.freeze + attr_reader :redirected_path + def wiki? - set_project unless defined?(@wiki) - @wiki + set_project unless defined?(@wiki) # rubocop:disable Gitlab/ModuleWithInstanceVariables + @wiki # rubocop:disable Gitlab/ModuleWithInstanceVariables end def project - set_project unless defined?(@project) - @project - end - - def redirected_path - @redirected_path + set_project unless defined?(@project) # rubocop:disable Gitlab/ModuleWithInstanceVariables + @project # rubocop:disable Gitlab/ModuleWithInstanceVariables end def ssh_authentication_abilities @@ -69,6 +67,7 @@ module API private + # rubocop:disable Gitlab/ModuleWithInstanceVariables def set_project if params[:gl_repository] @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository]) @@ -77,6 +76,7 @@ module API @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project]) end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables # Project id to pass between components that don't share/don't have # access to the same filesystem mounts diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 721ed97bb6b..d8aca3304c5 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -40,7 +40,7 @@ module ExtractsPath def extract_ref(id) pair = ['', ''] - return pair unless @project + return pair unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables if id =~ /^(\h{40})(.+)/ # If the ref appears to be a SHA, we're done, just split the string @@ -104,6 +104,7 @@ module ExtractsPath # # Automatically renders `not_found!` if a valid tree path could not be # resolved (e.g., when a user inserts an invalid path or ref). + # rubocop:disable Gitlab/ModuleWithInstanceVariables def assign_ref_vars # assign allowed options allowed_options = ["filter_ref"] @@ -127,13 +128,18 @@ module ExtractsPath @hex_path = Digest::SHA1.hexdigest(@path) @logs_path = logs_file_project_ref_path(@project, @ref, @path) - rescue RuntimeError, NoMethodError, InvalidPathError render_404 end + # rubocop:enable Gitlab/ModuleWithInstanceVariables def tree - @tree ||= @repo.tree(@commit.id, @path) + @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + + def lfs_blob_ids + blob_ids = tree.blobs.map(&:id) + @lfs_blob_ids = Gitlab::Git::Blob.batch_lfs_pointers(@project.repository, blob_ids).map(&:id) # rubocop:disable Gitlab/ModuleWithInstanceVariables end private @@ -146,8 +152,8 @@ module ExtractsPath end def ref_names - return [] unless @project + return [] unless @project # rubocop:disable Gitlab/ModuleWithInstanceVariables - @ref_names ||= @project.repository.ref_names + @ref_names ||= @project.repository.ref_names # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/cache/request_cache.rb b/lib/gitlab/cache/request_cache.rb index 754a45c3257..ecc85f847d4 100644 --- a/lib/gitlab/cache/request_cache.rb +++ b/lib/gitlab/cache/request_cache.rb @@ -45,11 +45,13 @@ module Gitlab klass.prepend(extension) end + attr_accessor :request_cache_key_block + def request_cache_key(&block) if block_given? - @request_cache_key = block + self.request_cache_key_block = block else - @request_cache_key + request_cache_key_block end end diff --git a/lib/gitlab/ci/charts.rb b/lib/gitlab/ci/charts.rb index 7df7b542d91..525563a97f5 100644 --- a/lib/gitlab/ci/charts.rb +++ b/lib/gitlab/ci/charts.rb @@ -6,7 +6,7 @@ module Gitlab query .group("DATE(#{::Ci::Pipeline.table_name}.created_at)") .count(:created_at) - .transform_keys { |date| date.strftime(@format) } + .transform_keys { |date| date.strftime(@format) } # rubocop:disable Gitlab/ModuleWithInstanceVariables end def interval_step diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb index 68b6742385a..db47c2f6185 100644 --- a/lib/gitlab/ci/config/entry/configurable.rb +++ b/lib/gitlab/ci/config/entry/configurable.rb @@ -29,15 +29,15 @@ module Gitlab self.class.nodes.each do |key, factory| factory - .value(@config[key]) + .value(config[key]) .with(key: key, parent: self) - @entries[key] = factory.create! + entries[key] = factory.create! end yield if block_given? - @entries.each_value do |entry| + entries.each_value do |entry| entry.compose!(deps) end end @@ -59,13 +59,13 @@ module Gitlab def helpers(*nodes) nodes.each do |symbol| define_method("#{symbol}_defined?") do - @entries[symbol]&.specified? + entries[symbol]&.specified? end define_method("#{symbol}_value") do - return unless @entries[symbol] && @entries[symbol].valid? + return unless entries[symbol] && entries[symbol].valid? - @entries[symbol].value + entries[symbol].value end end end diff --git a/lib/gitlab/ci/config/entry/node.rb b/lib/gitlab/ci/config/entry/node.rb index c868943c42e..1fba0b2db0b 100644 --- a/lib/gitlab/ci/config/entry/node.rb +++ b/lib/gitlab/ci/config/entry/node.rb @@ -90,6 +90,12 @@ module Gitlab def self.aspects @aspects ||= [] end + + private + + def entries + @entries + end end end end diff --git a/lib/gitlab/ci/config/entry/validatable.rb b/lib/gitlab/ci/config/entry/validatable.rb index 5ced778d311..e45787773a8 100644 --- a/lib/gitlab/ci/config/entry/validatable.rb +++ b/lib/gitlab/ci/config/entry/validatable.rb @@ -13,7 +13,7 @@ module Gitlab end def errors - @validator.messages + descendants.flat_map(&:errors) + @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Gitlab/ModuleWithInstanceVariables end class_methods do diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index b9099ce256a..76aee5a3deb 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -4,11 +4,11 @@ module Gitlab attr_reader :merge_request, :resolver def initialize(merge_request) - source_repo = merge_request.source_project.repository.raw our_commit = merge_request.source_branch_head.raw their_commit = merge_request.target_branch_head.raw target_repo = merge_request.target_project.repository.raw - @resolver = Gitlab::Git::Conflict::Resolver.new(source_repo, our_commit, target_repo, their_commit) + @source_repo = merge_request.source_project.repository.raw + @resolver = Gitlab::Git::Conflict::Resolver.new(target_repo, our_commit.id, their_commit.id) @merge_request = merge_request end @@ -18,7 +18,7 @@ module Gitlab target_branch: merge_request.target_branch, commit_message: commit_message || default_commit_message } - resolver.resolve_conflicts(user, files, args) + resolver.resolve_conflicts(@source_repo, user, files, args) ensure @merge_request.clear_memoized_shas end diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 642f0944354..91fd9cc7631 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -53,7 +53,7 @@ module Gitlab end def in_memory_application_settings - @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) + @in_memory_application_settings ||= ::ApplicationSetting.new(::ApplicationSetting.defaults) # rubocop:disable Gitlab/ModuleWithInstanceVariables rescue ActiveRecord::StatementInvalid, ActiveRecord::UnknownAttributeError # In case migrations the application_settings table is not created yet, # we fallback to a simple OpenStruct diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index ab115afcaa5..e3e3767cc75 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -56,7 +56,9 @@ module Gitlab end def allowed_ids - nil + @allowed_ids ||= allowed_ids_finder_class + .new(@options[:current_user], project_id: @project.id) + .execute.where(id: event_result_ids).pluck(:id) end def event_result_ids diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb index 58729d3ced8..dcbdf9a64b0 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -14,9 +14,9 @@ module Gitlab def stage_query query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id])) .join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id])) - .where(issue_table[:project_id].eq(@project.id)) + .where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Gitlab/ModuleWithInstanceVariables .where(issue_table[:deleted_at].eq(nil)) - .where(issue_table[:created_at].gteq(@options[:from])) + .where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables # Load merge_requests query = query.join(mr_table, Arel::Nodes::OuterJoin) diff --git a/lib/gitlab/cycle_analytics/code_event_fetcher.rb b/lib/gitlab/cycle_analytics/code_event_fetcher.rb index d5bf6149749..06357c9b377 100644 --- a/lib/gitlab/cycle_analytics/code_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/code_event_fetcher.rb @@ -1,8 +1,6 @@ module Gitlab module CycleAnalytics class CodeEventFetcher < BaseEventFetcher - include MergeRequestAllowed - def initialize(*args) @projections = [mr_table[:title], mr_table[:iid], @@ -20,6 +18,10 @@ module Gitlab def serialize(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event) end + + def allowed_ids_finder_class + MergeRequestsFinder + end end end end diff --git a/lib/gitlab/cycle_analytics/issue_allowed.rb b/lib/gitlab/cycle_analytics/issue_allowed.rb deleted file mode 100644 index a7652a70641..00000000000 --- a/lib/gitlab/cycle_analytics/issue_allowed.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Gitlab - module CycleAnalytics - module IssueAllowed - def allowed_ids - @allowed_ids ||= IssuesFinder.new(@options[:current_user], project_id: @project.id).execute.where(id: event_result_ids).pluck(:id) - end - end - end -end diff --git a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb index 3df9cbdcfce..1754f91dccb 100644 --- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -1,8 +1,6 @@ module Gitlab module CycleAnalytics class IssueEventFetcher < BaseEventFetcher - include IssueAllowed - def initialize(*args) @projections = [issue_table[:title], issue_table[:iid], @@ -18,6 +16,10 @@ module Gitlab def serialize(event) AnalyticsIssueSerializer.new(project: @project).represent(event) end + + def allowed_ids_finder_class + IssuesFinder + end end end end diff --git a/lib/gitlab/cycle_analytics/merge_request_allowed.rb b/lib/gitlab/cycle_analytics/merge_request_allowed.rb deleted file mode 100644 index 28f6db44759..00000000000 --- a/lib/gitlab/cycle_analytics/merge_request_allowed.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Gitlab - module CycleAnalytics - module MergeRequestAllowed - def allowed_ids - @allowed_ids ||= MergeRequestsFinder.new(@options[:current_user], project_id: @project.id).execute.where(id: event_result_ids).pluck(:id) - end - end - end -end diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index 9230894877f..086203b9ccc 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -18,6 +18,10 @@ module Gitlab private + def allowed_ids + nil + end + def merge_request_diff_commits @merge_request_diff_commits ||= MergeRequestDiffCommit diff --git a/lib/gitlab/cycle_analytics/production_helper.rb b/lib/gitlab/cycle_analytics/production_helper.rb index d693443bfa4..7a889b3877f 100644 --- a/lib/gitlab/cycle_analytics/production_helper.rb +++ b/lib/gitlab/cycle_analytics/production_helper.rb @@ -2,7 +2,9 @@ module Gitlab module CycleAnalytics module ProductionHelper def stage_query - super.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@options[:from])) + super + .where(mr_metrics_table[:first_deployed_to_production_at] + .gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/cycle_analytics/review_event_fetcher.rb b/lib/gitlab/cycle_analytics/review_event_fetcher.rb index 4c7b3f4467f..dada819a2a8 100644 --- a/lib/gitlab/cycle_analytics/review_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/review_event_fetcher.rb @@ -1,8 +1,6 @@ module Gitlab module CycleAnalytics class ReviewEventFetcher < BaseEventFetcher - include MergeRequestAllowed - def initialize(*args) @projections = [mr_table[:title], mr_table[:iid], @@ -14,9 +12,15 @@ module Gitlab super(*args) end + private + def serialize(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event) end + + def allowed_ids_finder_class + MergeRequestsFinder + end end end end diff --git a/lib/gitlab/cycle_analytics/staging_event_fetcher.rb b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb index 36c0260dbfe..2f014153ca5 100644 --- a/lib/gitlab/cycle_analytics/staging_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb @@ -22,6 +22,10 @@ module Gitlab private + def allowed_ids + nil + end + def serialize(event) AnalyticsBuildSerializer.new.represent(event['build']) end diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb index 7e492938eac..fd4a8832ec2 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb @@ -6,7 +6,7 @@ module Gitlab module Routable def full_path if route && route.path.present? - @full_path ||= route.path + @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables else update_route if persisted? @@ -30,7 +30,7 @@ module Gitlab def prepare_route route || build_route(source: self) route.path = build_full_path - @full_path = nil + @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/emoji.rb b/lib/gitlab/emoji.rb index e3e36b35ce9..89cf659bce4 100644 --- a/lib/gitlab/emoji.rb +++ b/lib/gitlab/emoji.rb @@ -31,8 +31,7 @@ module Gitlab end def emoji_unicode_version(name) - @emoji_unicode_versions_by_name ||= JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json'))) - @emoji_unicode_versions_by_name[name] + emoji_unicode_versions_by_name[name] end def normalize_emoji_name(name) @@ -56,5 +55,12 @@ module Gitlab ActionController::Base.helpers.content_tag('gl-emoji', emoji_info['moji'], title: emoji_info['description'], data: data) end + + private + + def emoji_unicode_versions_by_name + @emoji_unicode_versions_by_name ||= + JSON.parse(File.read(Rails.root.join('fixtures', 'emojis', 'emoji-unicode-version-map.json'))) + end end end diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb index de8cce41b6d..03e5c0fcd6f 100644 --- a/lib/gitlab/git/conflict/resolver.rb +++ b/lib/gitlab/git/conflict/resolver.rb @@ -5,38 +5,31 @@ module Gitlab ConflictSideMissing = Class.new(StandardError) ResolutionError = Class.new(StandardError) - def initialize(repository, our_commit, target_repository, their_commit) - @repository = repository - @our_commit = our_commit.rugged_commit + def initialize(target_repository, our_commit_oid, their_commit_oid) @target_repository = target_repository - @their_commit = their_commit.rugged_commit + @our_commit_oid = our_commit_oid + @their_commit_oid = their_commit_oid end def conflicts @conflicts ||= begin - target_index = @target_repository.rugged.merge_commits(@our_commit, @their_commit) + target_index = @target_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) # We don't need to do `with_repo_branch_commit` here, because the target # project always fetches source refs when creating merge request diffs. - target_index.conflicts.map do |conflict| - raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours] - - Gitlab::Git::Conflict::File.new( - @target_repository, - @our_commit.oid, - conflict, - target_index.merge_file(conflict[:ours][:path])[:data] - ) - end + conflict_files(@target_repository, target_index) end end - def resolve_conflicts(user, files, source_branch:, target_branch:, commit_message:) - @repository.with_repo_branch_commit(@target_repository, target_branch) do + def resolve_conflicts(source_repository, user, files, source_branch:, target_branch:, commit_message:) + source_repository.with_repo_branch_commit(@target_repository, target_branch) do + index = source_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) + conflicts = conflict_files(source_repository, index) + files.each do |file_params| - conflict_file = conflict_for_path(file_params[:old_path], file_params[:new_path]) + conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path]) - write_resolved_file_to_index(conflict_file, file_params) + write_resolved_file_to_index(source_repository, index, conflict_file, file_params) end unless index.conflicts.empty? @@ -47,14 +40,14 @@ module Gitlab commit_params = { message: commit_message, - parents: [@our_commit, @their_commit].map(&:oid) + parents: [@our_commit_oid, @their_commit_oid] } - @repository.commit_index(user, source_branch, index, commit_params) + source_repository.commit_index(user, source_branch, index, commit_params) end end - def conflict_for_path(old_path, new_path) + def conflict_for_path(conflicts, old_path, new_path) conflicts.find do |conflict| conflict.their_path == old_path && conflict.our_path == new_path end @@ -62,15 +55,20 @@ module Gitlab private - # We can only write when getting the merge index from the source - # project, because we will write to that project. We don't use this all - # the time because this fetches a ref into the source project, which - # isn't needed for reading. - def index - @index ||= @repository.rugged.merge_commits(@our_commit, @their_commit) + def conflict_files(repository, index) + index.conflicts.map do |conflict| + raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours] + + Gitlab::Git::Conflict::File.new( + repository, + @our_commit_oid, + conflict, + index.merge_file(conflict[:ours][:path])[:data] + ) + end end - def write_resolved_file_to_index(file, params) + def write_resolved_file_to_index(repository, index, file, params) if params[:sections] resolved_lines = file.resolve_lines(params[:sections]) new_file = resolved_lines.map { |line| line[:full_line] }.join("\n") @@ -82,7 +80,8 @@ module Gitlab our_path = file.our_path - index.add(path: our_path, oid: @repository.rugged.write(new_file, :blob), mode: file.our_mode) + oid = repository.rugged.write(new_file, :blob) + index.add(path: our_path, oid: oid, mode: file.our_mode) index.conflict_remove(our_path) end end diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb new file mode 100644 index 00000000000..d948d7895ed --- /dev/null +++ b/lib/gitlab/git/gitlab_projects.rb @@ -0,0 +1,258 @@ +module Gitlab + module Git + class GitlabProjects + include Gitlab::Git::Popen + + # Absolute path to directory where repositories are stored. + # Example: /home/git/repositories + attr_reader :shard_path + + # Relative path is a directory name for repository with .git at the end. + # Example: gitlab-org/gitlab-test.git + attr_reader :repository_relative_path + + # Absolute path to the repository. + # Example: /home/git/repositorities/gitlab-org/gitlab-test.git + attr_reader :repository_absolute_path + + # This is the path at which the gitlab-shell hooks directory can be found. + # It's essential for integration between git and GitLab proper. All new + # repositories should have their hooks directory symlinked here. + attr_reader :global_hooks_path + + attr_reader :logger + + def initialize(shard_path, repository_relative_path, global_hooks_path:, logger:) + @shard_path = shard_path + @repository_relative_path = repository_relative_path + + @logger = logger + @global_hooks_path = global_hooks_path + @repository_absolute_path = File.join(shard_path, repository_relative_path) + @output = StringIO.new + end + + def output + io = @output.dup + io.rewind + io.read + end + + def rm_project + logger.info "Removing repository <#{repository_absolute_path}>." + FileUtils.rm_rf(repository_absolute_path) + end + + # Move repository from one directory to another + # + # Example: gitlab/gitlab-ci.git -> randx/six.git + # + # Won't work if target namespace directory does not exist + # + def mv_project(new_path) + new_absolute_path = File.join(shard_path, new_path) + + # verify that the source repo exists + unless File.exist?(repository_absolute_path) + logger.error "mv-project failed: source path <#{repository_absolute_path}> does not exist." + return false + end + + # ...and that the target repo does not exist + if File.exist?(new_absolute_path) + logger.error "mv-project failed: destination path <#{new_absolute_path}> already exists." + return false + end + + logger.info "Moving repository from <#{repository_absolute_path}> to <#{new_absolute_path}>." + FileUtils.mv(repository_absolute_path, new_absolute_path) + end + + # Import project via git clone --bare + # URL must be publicly cloneable + def import_project(source, timeout) + # Skip import if repo already exists + return false if File.exist?(repository_absolute_path) + + masked_source = mask_password_in_url(source) + + logger.info "Importing project from <#{masked_source}> to <#{repository_absolute_path}>." + cmd = %W(git clone --bare -- #{source} #{repository_absolute_path}) + + success = run_with_timeout(cmd, timeout, nil) + + unless success + logger.error("Importing project from <#{masked_source}> to <#{repository_absolute_path}> failed.") + FileUtils.rm_rf(repository_absolute_path) + return false + end + + Gitlab::Git::Repository.create_hooks(repository_absolute_path, global_hooks_path) + + # The project was imported successfully. + # Remove the origin URL since it may contain password. + remove_origin_in_repo + + true + end + + def fork_repository(new_shard_path, new_repository_relative_path) + from_path = repository_absolute_path + to_path = File.join(new_shard_path, new_repository_relative_path) + + # The repository cannot already exist + if File.exist?(to_path) + logger.error "fork-repository failed: destination repository <#{to_path}> already exists." + return false + end + + # Ensure the namepsace / hashed storage directory exists + FileUtils.mkdir_p(File.dirname(to_path), mode: 0770) + + logger.info "Forking repository from <#{from_path}> to <#{to_path}>." + cmd = %W(git clone --bare --no-local -- #{from_path} #{to_path}) + + run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path) + end + + def fetch_remote(name, timeout, force:, tags:, ssh_key: nil, known_hosts: nil) + tags_option = tags ? '--tags' : '--no-tags' + + logger.info "Fetching remote #{name} for repository #{repository_absolute_path}." + cmd = %W(git fetch #{name} --prune --quiet) + cmd << '--force' if force + cmd << tags_option + + setup_ssh_auth(ssh_key, known_hosts) do |env| + success = run_with_timeout(cmd, timeout, repository_absolute_path, env) + + unless success + logger.error "Fetching remote #{name} for repository #{repository_absolute_path} failed." + end + + success + end + end + + def push_branches(remote_name, timeout, force, branch_names) + logger.info "Pushing branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" + cmd = %w(git push) + cmd << '--force' if force + cmd += %W(-- #{remote_name}).concat(branch_names) + + success = run_with_timeout(cmd, timeout, repository_absolute_path) + + unless success + logger.error("Pushing branches to remote #{remote_name} failed.") + end + + success + end + + def delete_remote_branches(remote_name, branch_names) + branches = branch_names.map { |branch_name| ":#{branch_name}" } + + logger.info "Pushing deleted branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" + cmd = %W(git push -- #{remote_name}).concat(branches) + + success = run(cmd, repository_absolute_path) + + unless success + logger.error("Pushing deleted branches to remote #{remote_name} failed.") + end + + success + end + + protected + + def run(*args) + output, exitstatus = popen(*args) + @output << output + + exitstatus&.zero? + end + + def run_with_timeout(*args) + output, exitstatus = popen_with_timeout(*args) + @output << output + + exitstatus&.zero? + rescue Timeout::Error + @output.puts('Timed out') + + false + end + + def mask_password_in_url(url) + result = URI(url) + result.password = "*****" unless result.password.nil? + result.user = "*****" unless result.user.nil? # it's needed for oauth access_token + result + rescue + url + end + + def remove_origin_in_repo + cmd = %w(git remote rm origin) + run(cmd, repository_absolute_path) + end + + # Builds a small shell script that can be used to execute SSH with a set of + # custom options. + # + # Options are expanded as `'-oKey="Value"'`, so SSH will correctly interpret + # paths with spaces in them. We trust the user not to embed single or double + # quotes in the key or value. + def custom_ssh_script(options = {}) + args = options.map { |k, v| %Q{'-o#{k}="#{v}"'} }.join(' ') + + [ + "#!/bin/sh", + "exec ssh #{args} \"$@\"" + ].join("\n") + end + + # Known hosts data and private keys can be passed to gitlab-shell in the + # environment. If present, this method puts them into temporary files, writes + # a script that can substitute as `ssh`, setting the options to respect those + # files, and yields: { "GIT_SSH" => "/tmp/myScript" } + def setup_ssh_auth(key, known_hosts) + options = {} + + if key + key_file = Tempfile.new('gitlab-shell-key-file') + key_file.chmod(0o400) + key_file.write(key) + key_file.close + + options['IdentityFile'] = key_file.path + options['IdentitiesOnly'] = 'yes' + end + + if known_hosts + known_hosts_file = Tempfile.new('gitlab-shell-known-hosts') + known_hosts_file.chmod(0o400) + known_hosts_file.write(known_hosts) + known_hosts_file.close + + options['StrictHostKeyChecking'] = 'yes' + options['UserKnownHostsFile'] = known_hosts_file.path + end + + return yield({}) if options.empty? + + script = Tempfile.new('gitlab-shell-ssh-wrapper') + script.chmod(0o755) + script.write(custom_ssh_script(options)) + script.close + + yield('GIT_SSH' => script.path) + ensure + key_file&.close! + known_hosts_file&.close! + script&.close! + end + end + end +end diff --git a/lib/gitlab/git/popen.rb b/lib/gitlab/git/popen.rb index d41fe78daa1..1ccca13ce2f 100644 --- a/lib/gitlab/git/popen.rb +++ b/lib/gitlab/git/popen.rb @@ -16,8 +16,8 @@ module Gitlab vars['PWD'] = path options = { chdir: path } - @cmd_output = "" - @cmd_status = 0 + cmd_output = "" + cmd_status = 0 Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| yield(stdin) if block_given? stdin.close @@ -25,14 +25,14 @@ module Gitlab if lazy_block return lazy_block.call(stdout.lazy) else - @cmd_output << stdout.read + cmd_output << stdout.read end - @cmd_output << stderr.read - @cmd_status = wait_thr.value.exitstatus + cmd_output << stderr.read + cmd_status = wait_thr.value.exitstatus end - [@cmd_output, @cmd_status] + [cmd_output, cmd_status] end def popen_with_timeout(cmd, timeout, path, vars = {}) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 369bb16f719..848a782446a 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -39,10 +39,31 @@ module Gitlab repo = Rugged::Repository.init_at(repo_path, bare) repo.close - if symlink_hooks_to.present? - hooks_path = File.join(repo_path, 'hooks') - FileUtils.rm_rf(hooks_path) - FileUtils.ln_s(symlink_hooks_to, hooks_path) + create_hooks(repo_path, symlink_hooks_to) if symlink_hooks_to.present? + + true + end + + def create_hooks(repo_path, global_hooks_path) + local_hooks_path = File.join(repo_path, 'hooks') + real_local_hooks_path = :not_found + + begin + real_local_hooks_path = File.realpath(local_hooks_path) + rescue Errno::ENOENT + # real_local_hooks_path == :not_found + end + + # Do nothing if hooks already exist + unless real_local_hooks_path == File.realpath(global_hooks_path) + # Move the existing hooks somewhere safe + FileUtils.mv( + local_hooks_path, + "#{local_hooks_path}.old.#{Time.now.to_i}" + ) if File.exist?(local_hooks_path) + + # Create the hooks symlink + FileUtils.ln_sf(global_hooks_path, local_hooks_path) end true diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index 90942774a2e..0135b3c6f22 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -32,7 +32,7 @@ module Gitlab def execute(cmd) output, status = Gitlab::Popen.popen(cmd) - @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? + @shared.error(Gitlab::ImportExport::Error.new(output.to_s)) unless status.zero? # rubocop:disable Gitlab/ModuleWithInstanceVariables status.zero? end diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index 3945df27eed..84ee94e38e4 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -36,10 +36,6 @@ module Gitlab ldap_config.block_auto_created_users end - def sync_profile_from_provider? - true - end - def allowed? Gitlab::LDAP::Access.allowed?(gl_user) end diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index bdf7910b7c7..6ea132fc5bf 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -154,6 +154,7 @@ module Gitlab # When enabled this should be set before being used as the usual pattern # "@foo ||= bar" is _not_ thread-safe. + # rubocop:disable Gitlab/ModuleWithInstanceVariables def pool if influx_metrics_enabled? if @pool.nil? @@ -170,6 +171,7 @@ module Gitlab @pool end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/metrics/prometheus.rb b/lib/gitlab/metrics/prometheus.rb index 09103b4ca2d..b0b8e8436db 100644 --- a/lib/gitlab/metrics/prometheus.rb +++ b/lib/gitlab/metrics/prometheus.rb @@ -4,6 +4,7 @@ module Gitlab module Metrics module Prometheus include Gitlab::CurrentSettings + include Gitlab::Utils::StrongMemoize REGISTRY_MUTEX = Mutex.new PROVIDER_MUTEX = Mutex.new @@ -17,16 +18,18 @@ module Gitlab end def prometheus_metrics_enabled? - return @prometheus_metrics_enabled if defined?(@prometheus_metrics_enabled) - - @prometheus_metrics_enabled = prometheus_metrics_enabled_unmemoized + strong_memoize(:prometheus_metrics_enabled) do + prometheus_metrics_enabled_unmemoized + end end def registry - return @registry if @registry - - REGISTRY_MUTEX.synchronize do - @registry ||= ::Prometheus::Client.registry + strong_memoize(:registry) do + REGISTRY_MUTEX.synchronize do + strong_memoize(:registry) do + ::Prometheus::Client.registry + end + end end end diff --git a/lib/gitlab/o_auth/provider.rb b/lib/gitlab/o_auth/provider.rb index ac9d66c836d..657db29c85a 100644 --- a/lib/gitlab/o_auth/provider.rb +++ b/lib/gitlab/o_auth/provider.rb @@ -19,6 +19,18 @@ module Gitlab name.to_s.start_with?('ldap') end + def self.sync_profile_from_provider?(provider) + return true if ldap_provider?(provider) + + providers = Gitlab.config.omniauth.sync_profile_from_provider + + if providers.is_a?(Array) + providers.include?(provider) + else + providers + end + end + def self.config_for(name) name = name.to_s if ldap_provider?(name) diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 552133234a3..d33f33d192f 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -12,7 +12,7 @@ module Gitlab def initialize(auth_hash) self.auth_hash = auth_hash - update_profile if sync_profile_from_provider? + update_profile add_or_update_user_identities end @@ -195,29 +195,31 @@ module Gitlab end def sync_profile_from_provider? - providers = Gitlab.config.omniauth.sync_profile_from_provider - - if providers.is_a?(Array) - providers.include?(auth_hash.provider) - else - providers - end + Gitlab::OAuth::Provider.sync_profile_from_provider?(auth_hash.provider) end def update_profile - user_synced_attributes_metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata - - UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key| - if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key) - gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend - user_synced_attributes_metadata.set_attribute_synced(key, true) - else - user_synced_attributes_metadata.set_attribute_synced(key, false) + return unless sync_profile_from_provider? || creating_linked_ldap_user? + + metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata + + if sync_profile_from_provider? + UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key| + if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key) + gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend + metadata.set_attribute_synced(key, true) + else + metadata.set_attribute_synced(key, false) + end end + + metadata.provider = auth_hash.provider end - user_synced_attributes_metadata.provider = auth_hash.provider - gl_user.user_synced_attributes_metadata = user_synced_attributes_metadata + if creating_linked_ldap_user? && gl_user.email == ldap_person.email.first + metadata.set_attribute_synced(:email, true) + metadata.provider = ldap_person.provider + end end def log diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index a22a63665be..9cdd3d22f18 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -66,7 +66,7 @@ module Gitlab # Init new repository # # storage - project's storage name - # name - project path with namespace + # name - project disk path # # Ex. # add_repository("/path/to/storage", "gitlab/gitlab-ci") @@ -94,23 +94,28 @@ module Gitlab # Import repository # # storage - project's storage path - # name - project path with namespace + # name - project disk path + # url - URL to import from # # Ex. - # import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://github.com/randx/six.git") + # import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://gitlab.com/gitlab-org/gitlab-test.git") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def import_repository(storage, name, url) # The timeout ensures the subprocess won't hang forever - cmd = [gitlab_shell_projects_path, 'import-project', - storage, "#{name}.git", url, "#{Gitlab.config.gitlab_shell.git_timeout}"] - gitlab_shell_fast_execute_raise_error(cmd) + cmd = gitlab_projects(storage, "#{name}.git") + success = cmd.import_project(url, git_timeout) + + raise Error, cmd.output unless success + + success end # Fetch remote for repository # # repository - an instance of Git::Repository # remote - remote name + # ssh_auth - SSH known_hosts data and a private key to use for public-key authentication # forced - should we use --force flag? # no_tags - should we use --no-tags flag? # @@ -131,16 +136,15 @@ module Gitlab # Move repository # storage - project's storage path - # path - project path with namespace - # new_path - new project path with namespace + # path - project disk path + # new_path - new project disk path # # Ex. # mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def mv_repository(storage, path, new_path) - gitlab_shell_fast_execute([gitlab_shell_projects_path, 'mv-project', - storage, "#{path}.git", "#{new_path}.git"]) + gitlab_projects(storage, "#{path}.git").mv_project("#{new_path}.git") end # Fork repository to new path @@ -154,30 +158,21 @@ module Gitlab # # Gitaly note: JV: not easy to migrate because this involves two Gitaly servers, not one. def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path) - gitlab_shell_fast_execute( - [ - gitlab_shell_projects_path, - 'fork-repository', - forked_from_storage, - "#{forked_from_disk_path}.git", - forked_to_storage, - "#{forked_to_disk_path}.git" - ] - ) + gitlab_projects(forked_from_storage, "#{forked_from_disk_path}.git") + .fork_repository(forked_to_storage, "#{forked_to_disk_path}.git") end # Remove repository from file system # # storage - project's storage path - # name - project path with namespace + # name - project disk path # # Ex. # remove_repository("/path/to/storage", "gitlab/gitlab-ci") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def remove_repository(storage, name) - gitlab_shell_fast_execute([gitlab_shell_projects_path, - 'rm-project', storage, "#{name}.git"]) + gitlab_projects(storage, "#{name}.git").rm_project end # Add new key to gitlab-shell @@ -311,6 +306,47 @@ module Gitlab end end + # Push branch to remote repository + # + # storage - project's storage path + # project_name - project's disk path + # remote_name - remote name + # branch_names - remote branch names to push + # forced - should we use --force flag + # + # Ex. + # push_remote_branches('/path/to/storage', 'gitlab-org/gitlab-test' 'upstream', ['feature']) + # + def push_remote_branches(storage, project_name, remote_name, branch_names, forced: true) + cmd = gitlab_projects(storage, "#{project_name}.git") + + success = cmd.push_branches(remote_name, git_timeout, forced, branch_names) + + raise Error, cmd.output unless success + + success + end + + # Delete branch from remote repository + # + # storage - project's storage path + # project_name - project's disk path + # remote_name - remote name + # branch_names - remote branch names + # + # Ex. + # delete_remote_branches('/path/to/storage', 'gitlab-org/gitlab-test', 'upstream', ['feature']) + # + def delete_remote_branches(storage, project_name, remote_name, branch_names) + cmd = gitlab_projects(storage, "#{project_name}.git") + + success = cmd.delete_remote_branches(remote_name, branch_names) + + raise Error, cmd.output unless success + + success + end + protected def gitlab_shell_path @@ -341,24 +377,35 @@ module Gitlab private - def local_fetch_remote(storage, name, remote, ssh_auth: nil, forced: false, no_tags: false) - args = [gitlab_shell_projects_path, 'fetch-remote', storage, name, remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] - args << '--force' if forced - args << '--no-tags' if no_tags + def gitlab_projects(shard_path, disk_path) + Gitlab::Git::GitlabProjects.new( + shard_path, + disk_path, + global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, + logger: Rails.logger + ) + end - vars = {} + def local_fetch_remote(storage_path, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false) + vars = { force: forced, tags: !no_tags } if ssh_auth&.ssh_import? if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? - vars['GITLAB_SHELL_SSH_KEY'] = ssh_auth.ssh_private_key + vars[:ssh_key] = ssh_auth.ssh_private_key end if ssh_auth.ssh_known_hosts.present? - vars['GITLAB_SHELL_KNOWN_HOSTS'] = ssh_auth.ssh_known_hosts + vars[:known_hosts] = ssh_auth.ssh_known_hosts end end - gitlab_shell_fast_execute_raise_error(args, vars) + cmd = gitlab_projects(storage_path, repository_relative_path) + + success = cmd.fetch_remote(remote, git_timeout, vars) + + raise Error, cmd.output unless success + + success end def gitlab_shell_fast_execute(cmd) @@ -394,6 +441,10 @@ module Gitlab Gitlab::GitalyClient::NamespaceService.new(storage) end + def git_timeout + Gitlab.config.gitlab_shell.git_timeout + end + def gitaly_migrate(method, &block) Gitlab::GitalyClient.migrate(method, &block) rescue GRPC::NotFound, GRPC::BadStatus => e diff --git a/lib/gitlab/slash_commands/presenters/issue_base.rb b/lib/gitlab/slash_commands/presenters/issue_base.rb index 341f2aabdd0..31c1e97efba 100644 --- a/lib/gitlab/slash_commands/presenters/issue_base.rb +++ b/lib/gitlab/slash_commands/presenters/issue_base.rb @@ -11,32 +11,36 @@ module Gitlab end def project - @resource.project + resource.project end def author - @resource.author + resource.author end def fields [ { title: "Assignee", - value: @resource.assignees.any? ? @resource.assignees.first.name : "_None_", + value: resource.assignees.any? ? resource.assignees.first.name : "_None_", short: true }, { title: "Milestone", - value: @resource.milestone ? @resource.milestone.title : "_None_", + value: resource.milestone ? resource.milestone.title : "_None_", short: true }, { title: "Labels", - value: @resource.labels.any? ? @resource.label_names.join(', ') : "_None_", + value: resource.labels.any? ? resource.label_names.join(', ') : "_None_", short: true } ] end + + private + + attr_reader :resource end end end diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake index 35ba729c156..247d7be7d78 100644 --- a/lib/tasks/gettext.rake +++ b/lib/tasks/gettext.rake @@ -23,6 +23,7 @@ namespace :gettext do desc 'Lint all po files in `locale/' task lint: :environment do require 'simple_po_parser' + require 'gitlab/utils' FastGettext.silence_errors files = Dir.glob(Rails.root.join('locale/*/gitlab.po')) diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index 8a63f486fa3..6723662703c 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -1,10 +1,13 @@ require 'rainbow/ext/string' +require 'gitlab/utils/strong_memoize' module Gitlab TaskFailedError = Class.new(StandardError) TaskAbortedByUserError = Class.new(StandardError) module TaskHelpers + include Gitlab::Utils::StrongMemoize + extend self # Ask if the user wants to continue @@ -105,16 +108,16 @@ module Gitlab end def gitlab_user? - return @is_gitlab_user unless @is_gitlab_user.nil? - - current_user = run_command(%w(whoami)).chomp - @is_gitlab_user = current_user == gitlab_user + strong_memoize(:is_gitlab_user) do + current_user = run_command(%w(whoami)).chomp + current_user == gitlab_user + end end def warn_user_is_not_gitlab - return if @warned_user_not_gitlab + return if gitlab_user? - unless gitlab_user? + strong_memoize(:warned_user_not_gitlab) do current_user = run_command(%w(whoami)).chomp puts " Warning ".color(:black).background(:yellow) @@ -122,8 +125,6 @@ module Gitlab puts " Things may work\/fail for the wrong reasons." puts " For correct results you should run this as user #{gitlab_user.color(:magenta)}." puts "" - - @warned_user_not_gitlab = true end end diff --git a/package.json b/package.json index a08f4784931..9e816e007ee 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "worker-loader": "^1.1.0" }, "devDependencies": { - "@gitlab-org/gitlab-svgs": "^1.2.0", + "@gitlab-org/gitlab-svgs": "^1.3.0", "babel-plugin-istanbul": "^4.1.5", "eslint": "^3.10.1", "eslint-config-airbnb-base": "^10.0.1", @@ -13,6 +13,27 @@ module QA end ## + # GitLab QA fabrication mechanisms + # + module Factory + autoload :Base, 'qa/factory/base' + + module Resource + autoload :Sandbox, 'qa/factory/resource/sandbox' + autoload :Group, 'qa/factory/resource/group' + autoload :Project, 'qa/factory/resource/project' + end + + module Repository + autoload :Push, 'qa/factory/repository/push' + end + + module Settings + autoload :HashedStorage, 'qa/factory/settings/hashed_storage' + end + end + + ## # GitLab QA Scenarios # module Scenario @@ -34,31 +55,6 @@ module QA autoload :Mattermost, 'qa/scenario/test/integration/mattermost' end end - - ## - # GitLab instance scenarios. - # - module Gitlab - module Group - autoload :Create, 'qa/scenario/gitlab/group/create' - end - - module Project - autoload :Create, 'qa/scenario/gitlab/project/create' - end - - module Repository - autoload :Push, 'qa/scenario/gitlab/repository/push' - end - - module Sandbox - autoload :Prepare, 'qa/scenario/gitlab/sandbox/prepare' - end - - module Admin - autoload :HashedStorage, 'qa/scenario/gitlab/admin/hashed_storage' - end - end end ## diff --git a/qa/qa/factory/base.rb b/qa/qa/factory/base.rb new file mode 100644 index 00000000000..7b951a99b69 --- /dev/null +++ b/qa/qa/factory/base.rb @@ -0,0 +1,16 @@ +module QA + module Factory + class Base + def self.fabricate!(*args) + new.tap do |factory| + yield factory if block_given? + return factory.fabricate!(*args) + end + end + + def fabricate!(*_args) + raise NotImplementedError + end + end + end +end diff --git a/qa/qa/factory/repository/push.rb b/qa/qa/factory/repository/push.rb new file mode 100644 index 00000000000..1d5375d8c76 --- /dev/null +++ b/qa/qa/factory/repository/push.rb @@ -0,0 +1,45 @@ +require "pry-byebug" + +module QA + module Factory + module Repository + class Push < Factory::Base + PAGE_REGEX_CHECK = + %r{\/#{Runtime::Namespace.sandbox_name}\/qa-test[^\/]+\/{1}[^\/]+\z}.freeze + + attr_writer :file_name, + :file_content, + :commit_message, + :branch_name + + def initialize + @file_name = 'file.txt' + @file_content = '# This is test project' + @commit_message = "Add #{@file_name}" + @branch_name = 'master' + end + + def fabricate! + Git::Repository.perform do |repository| + repository.location = Page::Project::Show.act do + unless PAGE_REGEX_CHECK.match(current_path) + raise "To perform this scenario the current page should be project show." + end + + choose_repository_clone_http + repository_location + end + + repository.use_default_credentials + repository.clone + repository.configure_identity('GitLab QA', 'root@gitlab.com') + + repository.add_file(@file_name, @file_content) + repository.commit(@commit_message) + repository.push_changes(@branch_name) + end + end + end + end + end +end diff --git a/qa/qa/factory/resource/group.rb b/qa/qa/factory/resource/group.rb new file mode 100644 index 00000000000..a081cd94d39 --- /dev/null +++ b/qa/qa/factory/resource/group.rb @@ -0,0 +1,23 @@ +module QA + module Factory + module Resource + class Group < Factory::Base + attr_writer :path, :description + + def initialize + @path = Runtime::Namespace.name + @description = "QA test run at #{Runtime::Namespace.time}" + end + + def fabricate! + Page::Group::New.perform do |group| + group.set_path(@path) + group.set_description(@description) + group.set_visibility('Private') + group.create + end + end + end + end + end +end diff --git a/qa/qa/factory/resource/project.rb b/qa/qa/factory/resource/project.rb new file mode 100644 index 00000000000..64fcfb084bb --- /dev/null +++ b/qa/qa/factory/resource/project.rb @@ -0,0 +1,40 @@ +require 'securerandom' + +module QA + module Factory + module Resource + class Project < Factory::Base + attr_writer :description + + def name=(name) + @name = "#{name}-#{SecureRandom.hex(8)}" + end + + def fabricate! + Factory::Resource::Sandbox.fabricate! + + Page::Group::Show.perform do |page| + if page.has_subgroup?(Runtime::Namespace.name) + page.go_to_subgroup(Runtime::Namespace.name) + else + page.go_to_new_subgroup + + Factory::Resource::Group.fabricate! do |group| + group.path = Runtime::Namespace.name + end + end + + page.go_to_new_project + end + + Page::Project::New.perform do |page| + page.choose_test_namespace + page.choose_name(@name) + page.add_description(@description) + page.create_new_project + end + end + end + end + end +end diff --git a/qa/qa/factory/resource/sandbox.rb b/qa/qa/factory/resource/sandbox.rb new file mode 100644 index 00000000000..fd2177915c5 --- /dev/null +++ b/qa/qa/factory/resource/sandbox.rb @@ -0,0 +1,28 @@ +module QA + module Factory + module Resource + ## + # Ensure we're in our sandbox namespace, either by navigating to it or by + # creating it if it doesn't yet exist. + # + class Sandbox < Factory::Base + def fabricate! + Page::Main::Menu.act { go_to_groups } + + Page::Dashboard::Groups.perform do |page| + if page.has_group?(Runtime::Namespace.sandbox_name) + page.go_to_group(Runtime::Namespace.sandbox_name) + else + page.go_to_new_group + + Resource::Group.fabricate! do |group| + group.path = Runtime::Namespace.sandbox_name + group.description = 'GitLab QA Sandbox' + end + end + end + end + end + end + end +end diff --git a/qa/qa/factory/settings/hashed_storage.rb b/qa/qa/factory/settings/hashed_storage.rb new file mode 100644 index 00000000000..eb3b28f2613 --- /dev/null +++ b/qa/qa/factory/settings/hashed_storage.rb @@ -0,0 +1,22 @@ +module QA + module Factory + module Settings + class HashedStorage < Factory::Base + def fabricate!(*traits) + raise ArgumentError unless traits.include?(:enabled) + + Page::Main::Login.act { sign_in_using_credentials } + Page::Main::Menu.act { go_to_admin_area } + Page::Admin::Menu.act { go_to_settings } + + Page::Admin::Settings.act do + enable_hashed_storage + save_settings + end + + QA::Page::Main::Menu.act { sign_out } + end + end + end + end +end diff --git a/qa/qa/page/group/show.rb b/qa/qa/page/group/show.rb index 8080deda675..100e71ae157 100644 --- a/qa/qa/page/group/show.rb +++ b/qa/qa/page/group/show.rb @@ -6,7 +6,13 @@ module QA click_link name end + def filter_by_name(name) + fill_in 'Filter by name...', with: name + end + def has_subgroup?(name) + filter_by_name(name) + page.has_link?(name) end diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index 6fb37fdfc7f..220bb45741b 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -51,6 +51,9 @@ module QA driver.browser.save_screenshot(path) end + # Keep only the screenshots generated from the last failing test suite + Capybara::Screenshot.prune_strategy = :keep_last_run + Capybara.configure do |config| config.default_driver = :chrome config.javascript_driver = :chrome diff --git a/qa/qa/runtime/scenario.rb b/qa/qa/runtime/scenario.rb index 7ef59046640..15d4112d347 100644 --- a/qa/qa/runtime/scenario.rb +++ b/qa/qa/runtime/scenario.rb @@ -6,13 +6,15 @@ module QA module Scenario extend self - attr_reader :attributes + def attributes + @attributes ||= {} + end def define(attribute, value) - (@attributes ||= {}).store(attribute.to_sym, value) + attributes.store(attribute.to_sym, value) define_singleton_method(attribute) do - @attributes[attribute.to_sym].tap do |value| + attributes[attribute.to_sym].tap do |value| if value.to_s.empty? raise ArgumentError, "Empty `#{attribute}` attribute!" end diff --git a/qa/qa/scenario/gitlab/admin/hashed_storage.rb b/qa/qa/scenario/gitlab/admin/hashed_storage.rb deleted file mode 100644 index 44604c6bc66..00000000000 --- a/qa/qa/scenario/gitlab/admin/hashed_storage.rb +++ /dev/null @@ -1,24 +0,0 @@ -module QA - module Scenario - module Gitlab - module Admin - class HashedStorage < Scenario::Template - def perform(*traits) - raise ArgumentError unless traits.include?(:enabled) - - Page::Main::Login.act { sign_in_using_credentials } - Page::Main::Menu.act { go_to_admin_area } - Page::Admin::Menu.act { go_to_settings } - - Page::Admin::Settings.act do - enable_hashed_storage - save_settings - end - - QA::Page::Main::Menu.act { sign_out } - end - end - end - end - end -end diff --git a/qa/qa/scenario/gitlab/group/create.rb b/qa/qa/scenario/gitlab/group/create.rb deleted file mode 100644 index 8e6c7c7ad80..00000000000 --- a/qa/qa/scenario/gitlab/group/create.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'securerandom' - -module QA - module Scenario - module Gitlab - module Group - class Create < Scenario::Template - attr_writer :path, :description - - def initialize - @path = Runtime::Namespace.name - @description = "QA test run at #{Runtime::Namespace.time}" - end - - def perform - Page::Group::New.perform do |group| - group.set_path(@path) - group.set_description(@description) - group.set_visibility('Private') - group.create - end - end - end - end - end - end -end diff --git a/qa/qa/scenario/gitlab/project/create.rb b/qa/qa/scenario/gitlab/project/create.rb deleted file mode 100644 index bb3b9e19c0f..00000000000 --- a/qa/qa/scenario/gitlab/project/create.rb +++ /dev/null @@ -1,42 +0,0 @@ -require 'securerandom' - -module QA - module Scenario - module Gitlab - module Project - class Create < Scenario::Template - attr_writer :description - - def name=(name) - @name = "#{name}-#{SecureRandom.hex(8)}" - end - - def perform - Scenario::Gitlab::Sandbox::Prepare.perform - - Page::Group::Show.perform do |page| - if page.has_subgroup?(Runtime::Namespace.name) - page.go_to_subgroup(Runtime::Namespace.name) - else - page.go_to_new_subgroup - - Scenario::Gitlab::Group::Create.perform do |group| - group.path = Runtime::Namespace.name - end - end - - page.go_to_new_project - end - - Page::Project::New.perform do |page| - page.choose_test_namespace - page.choose_name(@name) - page.add_description(@description) - page.create_new_project - end - end - end - end - end - end -end diff --git a/qa/qa/scenario/gitlab/repository/push.rb b/qa/qa/scenario/gitlab/repository/push.rb deleted file mode 100644 index b00ab0c313a..00000000000 --- a/qa/qa/scenario/gitlab/repository/push.rb +++ /dev/null @@ -1,47 +0,0 @@ -require "pry-byebug" - -module QA - module Scenario - module Gitlab - module Repository - class Push < Scenario::Template - PAGE_REGEX_CHECK = - %r{\/#{Runtime::Namespace.sandbox_name}\/qa-test[^\/]+\/{1}[^\/]+\z}.freeze - - attr_writer :file_name, - :file_content, - :commit_message, - :branch_name - - def initialize - @file_name = 'file.txt' - @file_content = '# This is test project' - @commit_message = "Add #{@file_name}" - @branch_name = 'master' - end - - def perform - Git::Repository.perform do |repository| - repository.location = Page::Project::Show.act do - unless PAGE_REGEX_CHECK.match(current_path) - raise "To perform this scenario the current page should be project show." - end - - choose_repository_clone_http - repository_location - end - - repository.use_default_credentials - repository.clone - repository.configure_identity('GitLab QA', 'root@gitlab.com') - - repository.add_file(@file_name, @file_content) - repository.commit(@commit_message) - repository.push_changes(@branch_name) - end - end - end - end - end - end -end diff --git a/qa/qa/scenario/gitlab/sandbox/prepare.rb b/qa/qa/scenario/gitlab/sandbox/prepare.rb deleted file mode 100644 index 990de456e20..00000000000 --- a/qa/qa/scenario/gitlab/sandbox/prepare.rb +++ /dev/null @@ -1,28 +0,0 @@ -module QA - module Scenario - module Gitlab - module Sandbox - # Ensure we're in our sandbox namespace, either by navigating to it or - # by creating it if it doesn't yet exist - class Prepare < Scenario::Template - def perform - Page::Main::Menu.act { go_to_groups } - - Page::Dashboard::Groups.perform do |page| - if page.has_group?(Runtime::Namespace.sandbox_name) - page.go_to_group(Runtime::Namespace.sandbox_name) - else - page.go_to_new_group - - Scenario::Gitlab::Group::Create.perform do |group| - group.path = Runtime::Namespace.sandbox_name - group.description = 'QA sandbox' - end - end - end - end - end - end - end - end -end diff --git a/qa/qa/specs/features/project/create_spec.rb b/qa/qa/specs/features/project/create_spec.rb index 0b3accb848d..61c19378ae0 100644 --- a/qa/qa/specs/features/project/create_spec.rb +++ b/qa/qa/specs/features/project/create_spec.rb @@ -4,7 +4,7 @@ module QA Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } - Scenario::Gitlab::Project::Create.perform do |project| + Factory::Resource::Project.fabricate! do |project| project.name = 'awesome-project' project.description = 'create awesome project test' end diff --git a/qa/qa/specs/features/repository/clone_spec.rb b/qa/qa/specs/features/repository/clone_spec.rb index c5c24622657..2adb7524a46 100644 --- a/qa/qa/specs/features/repository/clone_spec.rb +++ b/qa/qa/specs/features/repository/clone_spec.rb @@ -12,7 +12,7 @@ module QA Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } - Scenario::Gitlab::Project::Create.perform do |scenario| + Factory::Resource::Project.fabricate! do |scenario| scenario.name = 'project-with-code' scenario.description = 'project for git clone tests' end diff --git a/qa/qa/specs/features/repository/push_spec.rb b/qa/qa/specs/features/repository/push_spec.rb index ef29dfa2d2f..e47c769b015 100644 --- a/qa/qa/specs/features/repository/push_spec.rb +++ b/qa/qa/specs/features/repository/push_spec.rb @@ -5,12 +5,12 @@ module QA Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } - Scenario::Gitlab::Project::Create.perform do |scenario| + Factory::Resource::Project.fabricate! do |scenario| scenario.name = 'project_with_code' scenario.description = 'project with repository' end - Scenario::Gitlab::Repository::Push.perform do |scenario| + Factory::Repository::Push.fabricate! do |scenario| scenario.file_name = 'README.md' scenario.file_content = '# This is test project' scenario.commit_message = 'Add README.md' diff --git a/rubocop/cop/gitlab/module_with_instance_variables.rb b/rubocop/cop/gitlab/module_with_instance_variables.rb new file mode 100644 index 00000000000..5c9cde98512 --- /dev/null +++ b/rubocop/cop/gitlab/module_with_instance_variables.rb @@ -0,0 +1,63 @@ +module RuboCop + module Cop + module Gitlab + class ModuleWithInstanceVariables < RuboCop::Cop::Cop + MSG = <<~EOL.freeze + Do not use instance variables in a module. Please read this + for the rationale behind it: + + https://docs.gitlab.com/ee/development/module_with_instance_variables.html + EOL + + def on_module(node) + check_method_definition(node) + + # Not sure why some module would have an extra begin wrapping around + node.each_child_node(:begin) do |begin_node| + check_method_definition(begin_node) + end + end + + private + + def check_method_definition(node) + node.each_child_node(:def) do |definition| + # We allow this pattern: + # + # def f + # @f ||= true + # end + if only_ivar_or_assignment?(definition) + # We don't allow if any other ivar is used + definition.each_descendant(:ivar) do |offense| + add_offense(offense, :expression) + end + # We allow initialize method and single ivar + elsif !initialize_method?(definition) && !single_ivar?(definition) + definition.each_descendant(:ivar, :ivasgn) do |offense| + add_offense(offense, :expression) + end + end + end + end + + def only_ivar_or_assignment?(definition) + node = definition.child_nodes.last + + definition.child_nodes.size == 2 && + node.or_asgn_type? && node.child_nodes.first.ivasgn_type? + end + + def single_ivar?(definition) + node = definition.child_nodes.last + + definition.child_nodes.size == 2 && node.ivar_type? + end + + def initialize_method?(definition) + definition.children.first == :initialize + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 3e3b4c8349a..8aa82e9413d 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -8,6 +8,7 @@ require_relative 'cop/line_break_after_guard_clauses' require_relative 'cop/polymorphic_associations' require_relative 'cop/project_path_helper' require_relative 'cop/redirect_with_status' +require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/sidekiq_options_queue' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_concurrent_foreign_key' diff --git a/scripts/trigger-build-omnibus b/scripts/trigger-build-omnibus index 4ff0e8e10b7..85ea4aa74ac 100755 --- a/scripts/trigger-build-omnibus +++ b/scripts/trigger-build-omnibus @@ -21,6 +21,7 @@ module Omnibus if id puts "Triggered https://gitlab.com/#{Omnibus::PROJECT_PATH}/pipelines/#{id}" + puts "Waiting for downstream pipeline status" else raise "Trigger failed! The response from the trigger is: #{res.body}" end @@ -39,7 +40,9 @@ module Omnibus "ref" => ENV["OMNIBUS_BRANCH"] || "master", "variables[GITLAB_VERSION]" => ENV["CI_COMMIT_SHA"], "variables[ALTERNATIVE_SOURCES]" => true, - "variables[ee]" => ee? ? 'true' : 'false' + "variables[ee]" => ee? ? 'true' : 'false', + "variables[TRIGGERED_USER]" => ENV["GITLAB_USER_NAME"], + "variables[TRIGGER_SOURCE]" => "https://gitlab.com/gitlab-org/#{ENV['CI_PROJECT_NAME']}/-/jobs/#{ENV['CI_JOB_ID']}" } end @@ -63,14 +66,14 @@ module Omnibus def wait! loop do - raise 'Pipeline timeout!' if timeout? + raise "Pipeline timed out after waiting for #{duration} minutes!" if timeout? case status when :created, :pending, :running - puts "Waiting another #{INTERVAL} seconds ..." + print "." sleep INTERVAL when :success - puts "Omnibus pipeline succeeded!" + puts "Omnibus pipeline succeeded in #{duration} minutes!" break else raise "Omnibus pipeline did not succeed!" @@ -84,6 +87,10 @@ module Omnibus Time.now.to_i > (@start + MAX_DURATION) end + def duration + (Time.now.to_i - @start) / 60 + end + def status req = Net::HTTP::Get.new(@uri) req['PRIVATE-TOKEN'] = ENV['GITLAB_QA_ACCESS_TOKEN'] diff --git a/spec/factories/abuse_reports.rb b/spec/factories/abuse_reports.rb index 8f6422a7825..021971ac421 100644 --- a/spec/factories/abuse_reports.rb +++ b/spec/factories/abuse_reports.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :abuse_report do reporter factory: :user user diff --git a/spec/factories/appearances.rb b/spec/factories/appearances.rb index 860973024c9..5f9c57c0c8d 100644 --- a/spec/factories/appearances.rb +++ b/spec/factories/appearances.rb @@ -1,6 +1,6 @@ -# Read about factories at https://github.com/thoughtbot/factory_girl +# Read about factories at https://github.com/thoughtbot/factory_bot -FactoryGirl.define do +FactoryBot.define do factory :appearance do title "MepMep" description "This is my Community Edition instance" diff --git a/spec/factories/application_settings.rb b/spec/factories/application_settings.rb index aef65e724c2..3ecc90b6573 100644 --- a/spec/factories/application_settings.rb +++ b/spec/factories/application_settings.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :application_setting do end end diff --git a/spec/factories/award_emoji.rb b/spec/factories/award_emoji.rb index 4b858df52c9..a0abbbce686 100644 --- a/spec/factories/award_emoji.rb +++ b/spec/factories/award_emoji.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :award_emoji do name "thumbsup" user diff --git a/spec/factories/boards.rb b/spec/factories/boards.rb index 1ec042a6ab4..1e125237ae8 100644 --- a/spec/factories/boards.rb +++ b/spec/factories/boards.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :board do project diff --git a/spec/factories/broadcast_messages.rb b/spec/factories/broadcast_messages.rb index c2fdf89213a..9a65e7f8a3f 100644 --- a/spec/factories/broadcast_messages.rb +++ b/spec/factories/broadcast_messages.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :broadcast_message do message "MyText" starts_at 1.day.ago diff --git a/spec/factories/chat_names.rb b/spec/factories/chat_names.rb index 9a0be1a4598..56993e5da18 100644 --- a/spec/factories/chat_names.rb +++ b/spec/factories/chat_names.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :chat_name, class: ChatName do user factory: :user service factory: :service diff --git a/spec/factories/chat_teams.rb b/spec/factories/chat_teams.rb index ffedf69a69b..d048c46d6b6 100644 --- a/spec/factories/chat_teams.rb +++ b/spec/factories/chat_teams.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :chat_team, class: ChatTeam do sequence(:team_id) { |n| "abcdefghijklm#{n}" } namespace factory: :group diff --git a/spec/factories/ci/build_trace_section_names.rb b/spec/factories/ci/build_trace_section_names.rb index 1c16225f0e5..ce07e716dde 100644 --- a/spec/factories/ci/build_trace_section_names.rb +++ b/spec/factories/ci/build_trace_section_names.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_build_trace_section_name, class: Ci::BuildTraceSectionName do sequence(:name) { |n| "section_#{n}" } project factory: :project diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index c868525cbc0..dc1d88c92dc 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -1,6 +1,6 @@ include ActionDispatch::TestProcess -FactoryGirl.define do +FactoryBot.define do factory :ci_build, class: Ci::Build do name 'test' stage 'test' diff --git a/spec/factories/ci/group_variables.rb b/spec/factories/ci/group_variables.rb index 565ced9eb1a..64716842b12 100644 --- a/spec/factories/ci/group_variables.rb +++ b/spec/factories/ci/group_variables.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_group_variable, class: Ci::GroupVariable do sequence(:key) { |n| "VARIABLE_#{n}" } value 'VARIABLE_VALUE' diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb index 538dc422832..46afba2953c 100644 --- a/spec/factories/ci/job_artifacts.rb +++ b/spec/factories/ci/job_artifacts.rb @@ -1,6 +1,6 @@ include ActionDispatch::TestProcess -FactoryGirl.define do +FactoryBot.define do factory :ci_job_artifact, class: Ci::JobArtifact do job factory: :ci_build file_type :archive diff --git a/spec/factories/ci/pipeline_schedule.rb b/spec/factories/ci/pipeline_schedule.rb index 564fef6833b..b2b79807429 100644 --- a/spec/factories/ci/pipeline_schedule.rb +++ b/spec/factories/ci/pipeline_schedule.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_pipeline_schedule, class: Ci::PipelineSchedule do cron '0 1 * * *' cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE diff --git a/spec/factories/ci/pipeline_schedule_variables.rb b/spec/factories/ci/pipeline_schedule_variables.rb index ca64d1aada0..8d29118e310 100644 --- a/spec/factories/ci/pipeline_schedule_variables.rb +++ b/spec/factories/ci/pipeline_schedule_variables.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_pipeline_schedule_variable, class: Ci::PipelineScheduleVariable do sequence(:key) { |n| "VARIABLE_#{n}" } value 'VARIABLE_VALUE' diff --git a/spec/factories/ci/pipeline_variables.rb b/spec/factories/ci/pipeline_variables.rb index 7c1a7faec08..b18055d7b3a 100644 --- a/spec/factories/ci/pipeline_variables.rb +++ b/spec/factories/ci/pipeline_variables.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_pipeline_variable, class: Ci::PipelineVariable do sequence(:key) { |n| "VARIABLE_#{n}" } value 'VARIABLE_VALUE' diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index f994c2df821..51a767e5b93 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_empty_pipeline, class: Ci::Pipeline do source :push ref 'master' diff --git a/spec/factories/ci/runner_projects.rb b/spec/factories/ci/runner_projects.rb index fa76d0ecd8c..f605e90ceed 100644 --- a/spec/factories/ci/runner_projects.rb +++ b/spec/factories/ci/runner_projects.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_runner_project, class: Ci::RunnerProject do runner factory: :ci_runner project diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb index 88bb755d068..34b8b246d0f 100644 --- a/spec/factories/ci/runners.rb +++ b/spec/factories/ci/runners.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_runner, class: Ci::Runner do sequence(:description) { |n| "My runner#{n}" } diff --git a/spec/factories/ci/stages.rb b/spec/factories/ci/stages.rb index b2ded945738..25309033571 100644 --- a/spec/factories/ci/stages.rb +++ b/spec/factories/ci/stages.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_stage, class: Ci::LegacyStage do skip_create diff --git a/spec/factories/ci/trigger_requests.rb b/spec/factories/ci/trigger_requests.rb index 40b8848920e..0e9fc3d0014 100644 --- a/spec/factories/ci/trigger_requests.rb +++ b/spec/factories/ci/trigger_requests.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_trigger_request, class: Ci::TriggerRequest do trigger factory: :ci_trigger end diff --git a/spec/factories/ci/triggers.rb b/spec/factories/ci/triggers.rb index 3734c7040c0..742d9efba2d 100644 --- a/spec/factories/ci/triggers.rb +++ b/spec/factories/ci/triggers.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_trigger_without_token, class: Ci::Trigger do owner diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb index d8fd513ffcf..3d014b9b54f 100644 --- a/spec/factories/ci/variables.rb +++ b/spec/factories/ci/variables.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :ci_variable, class: Ci::Variable do sequence(:key) { |n| "VARIABLE_#{n}" } value 'VARIABLE_VALUE' diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb index fab37195113..d82fa8e34aa 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :cluster_applications_helm, class: Clusters::Applications::Helm do cluster factory: %i(cluster provided_by_gcp) diff --git a/spec/factories/clusters/applications/ingress.rb b/spec/factories/clusters/applications/ingress.rb index b103a980655..85f54a9d744 100644 --- a/spec/factories/clusters/applications/ingress.rb +++ b/spec/factories/clusters/applications/ingress.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :cluster_applications_ingress, class: Clusters::Applications::Ingress do cluster factory: %i(cluster provided_by_gcp) diff --git a/spec/factories/clusters/clusters.rb b/spec/factories/clusters/clusters.rb index 9e73a19e856..20d5580f0c2 100644 --- a/spec/factories/clusters/clusters.rb +++ b/spec/factories/clusters/clusters.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :cluster, class: Clusters::Cluster do user name 'test-cluster' diff --git a/spec/factories/clusters/platforms/kubernetes.rb b/spec/factories/clusters/platforms/kubernetes.rb index 8b3e6ff35fa..89f6ddebf6a 100644 --- a/spec/factories/clusters/platforms/kubernetes.rb +++ b/spec/factories/clusters/platforms/kubernetes.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :cluster_platform_kubernetes, class: Clusters::Platforms::Kubernetes do cluster namespace nil diff --git a/spec/factories/clusters/providers/gcp.rb b/spec/factories/clusters/providers/gcp.rb index a815410512a..a002ab28519 100644 --- a/spec/factories/clusters/providers/gcp.rb +++ b/spec/factories/clusters/providers/gcp.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :cluster_provider_gcp, class: Clusters::Providers::Gcp do cluster gcp_project_id 'test-gcp-project' diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb index abbe37df90e..ce5fbc343ee 100644 --- a/spec/factories/commit_statuses.rb +++ b/spec/factories/commit_statuses.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :commit_status, class: CommitStatus do name 'default' stage 'test' diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb index 4e2d8e8969e..84a8bc56640 100644 --- a/spec/factories/commits.rb +++ b/spec/factories/commits.rb @@ -1,6 +1,6 @@ require_relative '../support/repo_helpers' -FactoryGirl.define do +FactoryBot.define do factory :commit do transient do author nil diff --git a/spec/factories/container_repositories.rb b/spec/factories/container_repositories.rb index 3fcad9fd4b3..62a89a12ef5 100644 --- a/spec/factories/container_repositories.rb +++ b/spec/factories/container_repositories.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :container_repository do name 'test_container_image' project diff --git a/spec/factories/conversational_development_index_metrics.rb b/spec/factories/conversational_development_index_metrics.rb index 3806c43ba15..abf37fb861e 100644 --- a/spec/factories/conversational_development_index_metrics.rb +++ b/spec/factories/conversational_development_index_metrics.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :conversational_development_index_metric, class: ConversationalDevelopmentIndex::Metric do leader_issues 9.256 instance_issues 1.234 diff --git a/spec/factories/deploy_keys_projects.rb b/spec/factories/deploy_keys_projects.rb index 27cece487bd..30a6d468ed3 100644 --- a/spec/factories/deploy_keys_projects.rb +++ b/spec/factories/deploy_keys_projects.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :deploy_keys_project do deploy_key project diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 0dd1238d6e2..9d7d5e56611 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :deployment, class: Deployment do sha '97de212e80737a608d939f648d959671fb0a0142' ref 'master' diff --git a/spec/factories/emails.rb b/spec/factories/emails.rb index c9ab87a15aa..4dc7961060a 100644 --- a/spec/factories/emails.rb +++ b/spec/factories/emails.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :email do user email { generate(:email_alias) } diff --git a/spec/factories/environments.rb b/spec/factories/environments.rb index 9034476d094..b5db57d5148 100644 --- a/spec/factories/environments.rb +++ b/spec/factories/environments.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :environment, class: Environment do sequence(:name) { |n| "environment#{n}" } diff --git a/spec/factories/events.rb b/spec/factories/events.rb index ad9f7e2caef..ed275243ac9 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :event do project author factory: :user diff --git a/spec/factories/file_uploaders.rb b/spec/factories/file_uploaders.rb index 622571390d2..8404985bfea 100644 --- a/spec/factories/file_uploaders.rb +++ b/spec/factories/file_uploaders.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :file_uploader do skip_create diff --git a/spec/factories/fork_network_members.rb b/spec/factories/fork_network_members.rb index 509c4e1fa1c..850e3f158f1 100644 --- a/spec/factories/fork_network_members.rb +++ b/spec/factories/fork_network_members.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :fork_network_member do association :project association :fork_network diff --git a/spec/factories/fork_networks.rb b/spec/factories/fork_networks.rb index f42d36f3d19..813b1943eb2 100644 --- a/spec/factories/fork_networks.rb +++ b/spec/factories/fork_networks.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :fork_network do association :root_project, factory: :project end diff --git a/spec/factories/forked_project_links.rb b/spec/factories/forked_project_links.rb index 9b34651a4ae..bc59fea81ec 100644 --- a/spec/factories/forked_project_links.rb +++ b/spec/factories/forked_project_links.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :forked_project_link do association :forked_to_project, factory: [:project, :repository] association :forked_from_project, factory: [:project, :repository] diff --git a/spec/factories/gitaly/commit.rb b/spec/factories/gitaly/commit.rb index e7966cee77b..5034c3d0e48 100644 --- a/spec/factories/gitaly/commit.rb +++ b/spec/factories/gitaly/commit.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do sequence(:gitaly_commit_id) { Digest::SHA1.hexdigest(Time.now.to_f.to_s) } factory :gitaly_commit, class: Gitaly::GitCommit do diff --git a/spec/factories/gitaly/commit_author.rb b/spec/factories/gitaly/commit_author.rb index 341873a2002..aaf634ce08b 100644 --- a/spec/factories/gitaly/commit_author.rb +++ b/spec/factories/gitaly/commit_author.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :gitaly_commit_author, class: Gitaly::CommitAuthor do skip_create diff --git a/spec/factories/gpg_key_subkeys.rb b/spec/factories/gpg_key_subkeys.rb index 66ecb44d84b..57eaaee345f 100644 --- a/spec/factories/gpg_key_subkeys.rb +++ b/spec/factories/gpg_key_subkeys.rb @@ -1,6 +1,6 @@ require_relative '../support/gpg_helpers' -FactoryGirl.define do +FactoryBot.define do factory :gpg_key_subkey do gpg_key diff --git a/spec/factories/gpg_keys.rb b/spec/factories/gpg_keys.rb index 93218e5763e..b8aabf74221 100644 --- a/spec/factories/gpg_keys.rb +++ b/spec/factories/gpg_keys.rb @@ -1,6 +1,6 @@ require_relative '../support/gpg_helpers' -FactoryGirl.define do +FactoryBot.define do factory :gpg_key do key GpgHelpers::User1.public_key user diff --git a/spec/factories/gpg_signature.rb b/spec/factories/gpg_signature.rb index e9798ff6a41..4620caff823 100644 --- a/spec/factories/gpg_signature.rb +++ b/spec/factories/gpg_signature.rb @@ -1,6 +1,6 @@ require_relative '../support/gpg_helpers' -FactoryGirl.define do +FactoryBot.define do factory :gpg_signature do commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } project diff --git a/spec/factories/group_custom_attributes.rb b/spec/factories/group_custom_attributes.rb index 7ff5f376e8b..d2f45d5d3ce 100644 --- a/spec/factories/group_custom_attributes.rb +++ b/spec/factories/group_custom_attributes.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :group_custom_attribute do group sequence(:key) { |n| "key#{n}" } diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb index 32cbfe28a60..1c2214e9481 100644 --- a/spec/factories/group_members.rb +++ b/spec/factories/group_members.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :group_member do access_level { GroupMember::OWNER } group diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 52f76b094a3..1512f5a0e58 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :group, class: Group, parent: :namespace do sequence(:name) { |n| "group#{n}" } path { name.downcase.gsub(/\s/, '_') } diff --git a/spec/factories/identities.rb b/spec/factories/identities.rb index 26ef6f18698..122d0d42938 100644 --- a/spec/factories/identities.rb +++ b/spec/factories/identities.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :identity do provider 'ldapmain' extern_uid 'my-ldap-id' diff --git a/spec/factories/instance_configuration.rb b/spec/factories/instance_configuration.rb index 406c7c3caf1..31866a9c221 100644 --- a/spec/factories/instance_configuration.rb +++ b/spec/factories/instance_configuration.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :instance_configuration do skip_create end diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 7c3b80198f9..5ed6b017dee 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :issue do title { generate(:title) } author diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb index 3f7c794b14a..e6eb76f71d3 100644 --- a/spec/factories/keys.rb +++ b/spec/factories/keys.rb @@ -1,6 +1,6 @@ require_relative '../support/helpers/key_generator_helper' -FactoryGirl.define do +FactoryBot.define do factory :key do title key { Spec::Support::Helpers::KeyGeneratorHelper.new(1024).generate + ' dummy@gitlab.com' } diff --git a/spec/factories/label_links.rb b/spec/factories/label_links.rb index 3580174e873..007847d9cf4 100644 --- a/spec/factories/label_links.rb +++ b/spec/factories/label_links.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :label_link do label target factory: :issue diff --git a/spec/factories/label_priorities.rb b/spec/factories/label_priorities.rb index 7430466fc57..c4824faad53 100644 --- a/spec/factories/label_priorities.rb +++ b/spec/factories/label_priorities.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :label_priority do project label diff --git a/spec/factories/labels.rb b/spec/factories/labels.rb index 416317d677b..f759b6d499d 100644 --- a/spec/factories/labels.rb +++ b/spec/factories/labels.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do trait :base_label do title { generate(:label_title) } color "#990000" diff --git a/spec/factories/lfs_objects.rb b/spec/factories/lfs_objects.rb index 477fab9e964..8eb709022ce 100644 --- a/spec/factories/lfs_objects.rb +++ b/spec/factories/lfs_objects.rb @@ -1,6 +1,6 @@ include ActionDispatch::TestProcess -FactoryGirl.define do +FactoryBot.define do factory :lfs_object do sequence(:oid) { |n| "b68143e6463773b1b6c6fd009a76c32aeec041faff32ba2ed42fd7f708a%05x" % n } size 499013 diff --git a/spec/factories/lfs_objects_projects.rb b/spec/factories/lfs_objects_projects.rb index 1ed0355c8e4..c225387a5de 100644 --- a/spec/factories/lfs_objects_projects.rb +++ b/spec/factories/lfs_objects_projects.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :lfs_objects_project do lfs_object project diff --git a/spec/factories/lists.rb b/spec/factories/lists.rb index 48142d3c49b..210c58b21e9 100644 --- a/spec/factories/lists.rb +++ b/spec/factories/lists.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :list do board label diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index cc6cef63b47..40558c88d15 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :merge_request do title { generate(:title) } author diff --git a/spec/factories/merge_requests_closing_issues.rb b/spec/factories/merge_requests_closing_issues.rb index fdbdc00cad7..ee0606a72b6 100644 --- a/spec/factories/merge_requests_closing_issues.rb +++ b/spec/factories/merge_requests_closing_issues.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :merge_requests_closing_issues do issue merge_request diff --git a/spec/factories/milestones.rb b/spec/factories/milestones.rb index b5298b2f969..f95632e7187 100644 --- a/spec/factories/milestones.rb +++ b/spec/factories/milestones.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :milestone do title diff --git a/spec/factories/namespaces.rb b/spec/factories/namespaces.rb index 1b1fc4ce80d..f94b09cff15 100644 --- a/spec/factories/namespaces.rb +++ b/spec/factories/namespaces.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :namespace do sequence(:name) { |n| "namespace#{n}" } path { name.downcase.gsub(/\s/, '_') } diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 471bfb3213a..707ecbd6be5 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -2,7 +2,7 @@ require_relative '../support/repo_helpers' include ActionDispatch::TestProcess -FactoryGirl.define do +FactoryBot.define do factory :note do project note { generate(:title) } diff --git a/spec/factories/notification_settings.rb b/spec/factories/notification_settings.rb index e9171528d86..5116ef33f5d 100644 --- a/spec/factories/notification_settings.rb +++ b/spec/factories/notification_settings.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :notification_setting do source factory: :project user diff --git a/spec/factories/oauth_access_grants.rb b/spec/factories/oauth_access_grants.rb index 543b3e99274..9e6af24c4eb 100644 --- a/spec/factories/oauth_access_grants.rb +++ b/spec/factories/oauth_access_grants.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :oauth_access_grant do resource_owner_id { create(:user).id } application diff --git a/spec/factories/oauth_access_tokens.rb b/spec/factories/oauth_access_tokens.rb index a46bc1d8ce8..eabfd6cd830 100644 --- a/spec/factories/oauth_access_tokens.rb +++ b/spec/factories/oauth_access_tokens.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :oauth_access_token do resource_owner application diff --git a/spec/factories/oauth_applications.rb b/spec/factories/oauth_applications.rb index c7ede40f240..4427da1d6c7 100644 --- a/spec/factories/oauth_applications.rb +++ b/spec/factories/oauth_applications.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :oauth_application, class: 'Doorkeeper::Application', aliases: [:application] do sequence(:name) { |n| "OAuth App #{n}" } uid { Doorkeeper::OAuth::Helpers::UniqueToken.generate } diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb index 6d2e45f41ba..61b04708da2 100644 --- a/spec/factories/pages_domains.rb +++ b/spec/factories/pages_domains.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :pages_domain, class: 'PagesDomain' do domain 'my.domain.com' diff --git a/spec/factories/personal_access_tokens.rb b/spec/factories/personal_access_tokens.rb index 06acaff6cd0..1b12f84d7b8 100644 --- a/spec/factories/personal_access_tokens.rb +++ b/spec/factories/personal_access_tokens.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :personal_access_token do user token { SecureRandom.hex(50) } diff --git a/spec/factories/project_auto_devops.rb b/spec/factories/project_auto_devops.rb index 8d124dc2381..5ce1988c76f 100644 --- a/spec/factories/project_auto_devops.rb +++ b/spec/factories/project_auto_devops.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :project_auto_devops do project enabled true diff --git a/spec/factories/project_custom_attributes.rb b/spec/factories/project_custom_attributes.rb index 5eedeb86304..099d2d7ff19 100644 --- a/spec/factories/project_custom_attributes.rb +++ b/spec/factories/project_custom_attributes.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :project_custom_attribute do project sequence(:key) { |n| "key#{n}" } diff --git a/spec/factories/project_group_links.rb b/spec/factories/project_group_links.rb index e73cc05f9d7..d5ace9425a0 100644 --- a/spec/factories/project_group_links.rb +++ b/spec/factories/project_group_links.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :project_group_link do project group diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index accae636a3a..493b7bc021c 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :project_hook do url { generate(:url) } enable_ssl_verification false diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb index 9cf3a1e8e8a..4260f52498d 100644 --- a/spec/factories/project_members.rb +++ b/spec/factories/project_members.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :project_member do user project diff --git a/spec/factories/project_statistics.rb b/spec/factories/project_statistics.rb index 6c2ed7c6581..2d0f698475d 100644 --- a/spec/factories/project_statistics.rb +++ b/spec/factories/project_statistics.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :project_statistics do project diff --git a/spec/factories/project_wikis.rb b/spec/factories/project_wikis.rb index 38fcab7466d..89d8248f9f4 100644 --- a/spec/factories/project_wikis.rb +++ b/spec/factories/project_wikis.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :project_wiki do skip_create diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 4034e7905ad..d0f3911f730 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -1,6 +1,6 @@ require_relative '../support/test_env' -FactoryGirl.define do +FactoryBot.define do # Project without repository # # Project does not have bare repository. diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb index fe0cbfc4444..39460834d06 100644 --- a/spec/factories/protected_branches.rb +++ b/spec/factories/protected_branches.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :protected_branch do name project diff --git a/spec/factories/protected_tags.rb b/spec/factories/protected_tags.rb index 225588b23cc..df9c8b3cb63 100644 --- a/spec/factories/protected_tags.rb +++ b/spec/factories/protected_tags.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :protected_tag do name project diff --git a/spec/factories/releases.rb b/spec/factories/releases.rb index 74497dc82c0..d80c65cf8bb 100644 --- a/spec/factories/releases.rb +++ b/spec/factories/releases.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :release do tag "v1.1.0" description "Awesome release" diff --git a/spec/factories/sent_notifications.rb b/spec/factories/sent_notifications.rb index c2febf5b438..80872067233 100644 --- a/spec/factories/sent_notifications.rb +++ b/spec/factories/sent_notifications.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :sent_notification do project recipient factory: :user diff --git a/spec/factories/sequences.rb b/spec/factories/sequences.rb index c0232ba5bf6..f2b6e7a11f9 100644 --- a/spec/factories/sequences.rb +++ b/spec/factories/sequences.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do sequence(:username) { |n| "user#{n}" } sequence(:name) { |n| "John Doe#{n}" } sequence(:email) { |n| "user#{n}@example.org" } diff --git a/spec/factories/service_hooks.rb b/spec/factories/service_hooks.rb index e3f88ab8fcc..c907862b4f6 100644 --- a/spec/factories/service_hooks.rb +++ b/spec/factories/service_hooks.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :service_hook do url { generate(:url) } service diff --git a/spec/factories/services.rb b/spec/factories/services.rb index ccf63f3ffa4..4b0377967c7 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :service do project type 'Service' diff --git a/spec/factories/snippets.rb b/spec/factories/snippets.rb index 075bccd7f94..2ab9a56d255 100644 --- a/spec/factories/snippets.rb +++ b/spec/factories/snippets.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :snippet do author title { generate(:title) } diff --git a/spec/factories/spam_logs.rb b/spec/factories/spam_logs.rb index e369f9f13e9..a467f850a80 100644 --- a/spec/factories/spam_logs.rb +++ b/spec/factories/spam_logs.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :spam_log do user sequence(:source_ip) { |n| "42.42.42.#{n % 255}" } diff --git a/spec/factories/subscriptions.rb b/spec/factories/subscriptions.rb index 1ae7fc9f384..a4bc4e87b0a 100644 --- a/spec/factories/subscriptions.rb +++ b/spec/factories/subscriptions.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :subscription do user project diff --git a/spec/factories/system_hooks.rb b/spec/factories/system_hooks.rb index 841e1e293e8..9e00eeb6ef1 100644 --- a/spec/factories/system_hooks.rb +++ b/spec/factories/system_hooks.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :system_hook do url { generate(:url) } end diff --git a/spec/factories/system_note_metadata.rb b/spec/factories/system_note_metadata.rb index f487a2d7e4a..e913068da40 100644 --- a/spec/factories/system_note_metadata.rb +++ b/spec/factories/system_note_metadata.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :system_note_metadata do note action 'merge' diff --git a/spec/factories/timelogs.rb b/spec/factories/timelogs.rb index 6f1545418eb..af34b0681e2 100644 --- a/spec/factories/timelogs.rb +++ b/spec/factories/timelogs.rb @@ -1,6 +1,6 @@ -# Read about factories at https://github.com/thoughtbot/factory_girl +# Read about factories at https://github.com/thoughtbot/factory_bot -FactoryGirl.define do +FactoryBot.define do factory :timelog do time_spent 3600 user diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb index 4975befbfe3..6a6de665dd1 100644 --- a/spec/factories/todos.rb +++ b/spec/factories/todos.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :todo do project author diff --git a/spec/factories/trending_project.rb b/spec/factories/trending_project.rb index 246176611dc..f7c634fd21f 100644 --- a/spec/factories/trending_project.rb +++ b/spec/factories/trending_project.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do # TrendingProject factory :trending_project, class: 'TrendingProject' do project diff --git a/spec/factories/u2f_registrations.rb b/spec/factories/u2f_registrations.rb index df92b079581..26090b08966 100644 --- a/spec/factories/u2f_registrations.rb +++ b/spec/factories/u2f_registrations.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :u2f_registration do certificate { FFaker::BaconIpsum.characters(728) } key_handle { FFaker::BaconIpsum.characters(86) } diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb index e18f1a6bd4a..c39500faea1 100644 --- a/spec/factories/uploads.rb +++ b/spec/factories/uploads.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :upload do model { build(:project) } path { "uploads/-/system/project/avatar/avatar.jpg" } diff --git a/spec/factories/user_agent_details.rb b/spec/factories/user_agent_details.rb index 9763cc0cf15..7183a8e1140 100644 --- a/spec/factories/user_agent_details.rb +++ b/spec/factories/user_agent_details.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :user_agent_detail do ip_address '127.0.0.1' user_agent 'AppleWebKit/537.36' diff --git a/spec/factories/user_custom_attributes.rb b/spec/factories/user_custom_attributes.rb index 278cf290d4f..a184a2e0f17 100644 --- a/spec/factories/user_custom_attributes.rb +++ b/spec/factories/user_custom_attributes.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :user_custom_attribute do user sequence(:key) { |n| "key#{n}" } diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 8ace424f8af..e62e0b263ca 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :user, aliases: [:author, :assignee, :recipient, :owner, :resource_owner] do email { generate(:email) } name { generate(:name) } diff --git a/spec/factories/web_hook_log.rb b/spec/factories/web_hook_log.rb index 230b3f6b26e..17837260a4b 100644 --- a/spec/factories/web_hook_log.rb +++ b/spec/factories/web_hook_log.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :web_hook_log do web_hook factory: :project_hook trigger 'push_hooks' diff --git a/spec/factories/wiki_directories.rb b/spec/factories/wiki_directories.rb index 3b4cfc380b8..b105c82b19d 100644 --- a/spec/factories/wiki_directories.rb +++ b/spec/factories/wiki_directories.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :wiki_directory do skip_create diff --git a/spec/factories/wiki_pages.rb b/spec/factories/wiki_pages.rb index 4105f59e289..2335b5118dd 100644 --- a/spec/factories/wiki_pages.rb +++ b/spec/factories/wiki_pages.rb @@ -1,6 +1,6 @@ require 'ostruct' -FactoryGirl.define do +FactoryBot.define do factory :wiki_page do transient do attrs do diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb index 09b3c0b0994..66b71d0f556 100644 --- a/spec/factories_spec.rb +++ b/spec/factories_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'factories' do - FactoryGirl.factories.each do |factory| + FactoryBot.factories.each do |factory| describe "#{factory.name} factory" do it 'does not raise error when built' do expect { build(factory.name) }.not_to raise_error diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index e3bb16af38a..c1c54177167 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -13,8 +13,8 @@ describe "Admin Runners" do context "when there are runners" do before do - runner = FactoryGirl.create(:ci_runner, contacted_at: Time.now) - FactoryGirl.create(:ci_build, pipeline: pipeline, runner_id: runner.id) + runner = FactoryBot.create(:ci_runner, contacted_at: Time.now) + FactoryBot.create(:ci_build, pipeline: pipeline, runner_id: runner.id) visit admin_runners_path end @@ -25,8 +25,8 @@ describe "Admin Runners" do describe 'search' do before do - FactoryGirl.create :ci_runner, description: 'runner-foo' - FactoryGirl.create :ci_runner, description: 'runner-bar' + FactoryBot.create :ci_runner, description: 'runner-foo' + FactoryBot.create :ci_runner, description: 'runner-bar' end it 'shows correct runner when description matches' do @@ -62,11 +62,11 @@ describe "Admin Runners" do end describe "Runner show page" do - let(:runner) { FactoryGirl.create :ci_runner } + let(:runner) { FactoryBot.create :ci_runner } before do - @project1 = FactoryGirl.create(:project) - @project2 = FactoryGirl.create(:project) + @project1 = FactoryBot.create(:project) + @project2 = FactoryBot.create(:project) visit admin_runner_path(runner) end diff --git a/spec/features/admin/admin_system_info_spec.rb b/spec/features/admin/admin_system_info_spec.rb index 1fd1cda694a..5a989319d5b 100644 --- a/spec/features/admin/admin_system_info_spec.rb +++ b/spec/features/admin/admin_system_info_spec.rb @@ -18,8 +18,8 @@ describe 'Admin System Info' do it 'shows system info page' do expect(page).to have_content 'CPU 2 cores' - expect(page).to have_content 'Memory 4 GB / 16 GB' - expect(page).to have_content 'Disks' + expect(page).to have_content 'Memory Usage 4 GB / 16 GB' + expect(page).to have_content 'Disk Usage' expect(page).to have_content 'Uptime' end end @@ -33,8 +33,8 @@ describe 'Admin System Info' do it 'shows system info page with no CPU info' do expect(page).to have_content 'CPU Unable to collect CPU info' - expect(page).to have_content 'Memory 4 GB / 16 GB' - expect(page).to have_content 'Disks' + expect(page).to have_content 'Memory Usage 4 GB / 16 GB' + expect(page).to have_content 'Disk Usage' expect(page).to have_content 'Uptime' end end @@ -48,8 +48,8 @@ describe 'Admin System Info' do it 'shows system info page with no CPU info' do expect(page).to have_content 'CPU 2 cores' - expect(page).to have_content 'Memory Unable to collect memory info' - expect(page).to have_content 'Disks' + expect(page).to have_content 'Memory Usage Unable to collect memory info' + expect(page).to have_content 'Disk Usage' expect(page).to have_content 'Uptime' end end diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb index d2d0be35f1c..e9b375f4c94 100644 --- a/spec/features/group_variables_spec.rb +++ b/spec/features/group_variables_spec.rb @@ -24,7 +24,7 @@ feature 'Group variables', :js do expect(find(".variable-value")).to have_content('******') expect(find(".variable-protected")).to have_content('Yes') end - click_on 'Reveal Values' + click_on 'Reveal value' page.within('.variables-table') do expect(find(".variable-value")).to have_content('AAA123') end diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb index 93be3b066ee..ab896a310be 100644 --- a/spec/features/help_pages_spec.rb +++ b/spec/features/help_pages_spec.rb @@ -37,7 +37,7 @@ describe 'Help Pages' do context 'in a production environment with version check enabled', :js do before do allow(Rails.env).to receive(:production?) { true } - allow_any_instance_of(ApplicationSetting).to receive(:version_check_enabled) { true } + stub_application_setting(version_check_enabled: true) allow_any_instance_of(VersionCheck).to receive(:url) { '/version-check-url' } sign_in(create(:user)) @@ -56,9 +56,9 @@ describe 'Help Pages' do describe 'when help page is customized' do before do - allow_any_instance_of(ApplicationSetting).to receive(:help_page_hide_commercial_content?) { true } - allow_any_instance_of(ApplicationSetting).to receive(:help_page_text) { "My Custom Text" } - allow_any_instance_of(ApplicationSetting).to receive(:help_page_support_url) { "http://example.com/help" } + stub_application_setting(help_page_hide_commercial_content: true, + help_page_text: 'My Custom Text', + help_page_support_url: 'http://example.com/help') sign_in(create(:user)) visit help_path diff --git a/spec/features/projects/tree/tree_show_spec.rb b/spec/features/projects/tree/tree_show_spec.rb new file mode 100644 index 00000000000..c8a17871508 --- /dev/null +++ b/spec/features/projects/tree/tree_show_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +feature 'Projects tree' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + before do + project.add_master(user) + sign_in(user) + + visit project_tree_path(project, 'master') + end + + it 'renders tree table' do + expect(page).to have_selector('.tree-item') + expect(page).not_to have_selector('.label-lfs', text: 'LFS') + end + + context 'LFS' do + before do + visit project_tree_path(project, File.join('master', 'files/lfs')) + end + + it 'renders LFS badge on blob item' do + expect(page).to have_selector('.label-lfs', text: 'LFS') + end + end +end diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb index c78f7d0d9be..dde60c83536 100644 --- a/spec/features/variables_spec.rb +++ b/spec/features/variables_spec.rb @@ -65,14 +65,14 @@ describe 'Project variables', :js do expect(page).to have_content('******') end - click_button('Reveal Values') + click_button('Reveal values') page.within('.variables-table') do expect(page).to have_content('key') expect(page).to have_content('key value') end - click_button('Hide Values') + click_button('Hide values') page.within('.variables-table') do expect(page).to have_content('key') diff --git a/spec/helpers/runners_helper_spec.rb b/spec/helpers/runners_helper_spec.rb index 35f91b7decf..a4a483e68a8 100644 --- a/spec/helpers/runners_helper_spec.rb +++ b/spec/helpers/runners_helper_spec.rb @@ -2,17 +2,17 @@ require 'spec_helper' describe RunnersHelper do it "returns - not contacted yet" do - runner = FactoryGirl.build :ci_runner + runner = FactoryBot.build :ci_runner expect(runner_status_icon(runner)).to include("not connected yet") end it "returns offline text" do - runner = FactoryGirl.build(:ci_runner, contacted_at: 1.day.ago, active: true) + runner = FactoryBot.build(:ci_runner, contacted_at: 1.day.ago, active: true) expect(runner_status_icon(runner)).to include("Runner is offline") end it "returns online text" do - runner = FactoryGirl.build(:ci_runner, contacted_at: 1.second.ago, active: true) + runner = FactoryBot.build(:ci_runner, contacted_at: 1.second.ago, active: true) expect(runner_status_icon(runner)).to include("Runner is online") end end diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb index c358ccae9c3..d3b1be599dd 100644 --- a/spec/helpers/tree_helper_spec.rb +++ b/spec/helpers/tree_helper_spec.rb @@ -9,6 +9,7 @@ describe TreeHelper do before do @id = sha @project = project + @lfs_blob_ids = [] end it 'displays all entries without a warning' do diff --git a/spec/javascripts/activities_spec.js b/spec/javascripts/activities_spec.js index fc9be14df8f..7a9c539e9d0 100644 --- a/spec/javascripts/activities_spec.js +++ b/spec/javascripts/activities_spec.js @@ -1,7 +1,6 @@ /* eslint-disable no-unused-expressions, no-prototype-builtins, no-new, no-shadow, max-len */ import 'vendor/jquery.endless-scroll'; -import '~/pager'; import Activities from '~/activities'; (() => { diff --git a/spec/javascripts/behaviors/secret_values_spec.js b/spec/javascripts/behaviors/secret_values_spec.js new file mode 100644 index 00000000000..9eeae474e7d --- /dev/null +++ b/spec/javascripts/behaviors/secret_values_spec.js @@ -0,0 +1,146 @@ +import SecretValues from '~/behaviors/secret_values'; + +function generateFixtureMarkup(secrets, isRevealed) { + return ` + <div class="js-secret-container"> + ${secrets.map(secret => ` + <div class="js-secret-value-placeholder"> + *** + </div> + <div class="hide js-secret-value"> + ${secret} + </div> + `).join('')} + <button + class="js-secret-value-reveal-button" + data-secret-reveal-status="${isRevealed}" + > + ... + </button> + </div> + `; +} + +function setupSecretFixture(secrets, isRevealed) { + const wrapper = document.createElement('div'); + wrapper.innerHTML = generateFixtureMarkup(secrets, isRevealed); + + const secretValues = new SecretValues(wrapper.querySelector('.js-secret-container')); + secretValues.init(); + + return wrapper; +} + +describe('setupSecretValues', () => { + describe('with a single secret', () => { + const secrets = ['mysecret123']; + + it('should have correct "Reveal" label when values are hidden', () => { + const wrapper = setupSecretFixture(secrets, false); + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + + expect(revealButton.textContent).toEqual('Reveal value'); + }); + + it('should have correct "Hide" label when values are shown', () => { + const wrapper = setupSecretFixture(secrets, true); + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + + expect(revealButton.textContent).toEqual('Hide value'); + }); + + it('should value hidden initially', () => { + const wrapper = setupSecretFixture(secrets, false); + const values = wrapper.querySelectorAll('.js-secret-value'); + const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder'); + + expect(values.length).toEqual(1); + expect(values[0].classList.contains('hide')).toEqual(true); + expect(placeholders.length).toEqual(1); + expect(placeholders[0].classList.contains('hide')).toEqual(false); + }); + + it('should toggle value and placeholder', () => { + const wrapper = setupSecretFixture(secrets, false); + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + const values = wrapper.querySelectorAll('.js-secret-value'); + const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder'); + + revealButton.click(); + + expect(values.length).toEqual(1); + expect(values[0].classList.contains('hide')).toEqual(false); + expect(placeholders.length).toEqual(1); + expect(placeholders[0].classList.contains('hide')).toEqual(true); + + revealButton.click(); + + expect(values.length).toEqual(1); + expect(values[0].classList.contains('hide')).toEqual(true); + expect(placeholders.length).toEqual(1); + expect(placeholders[0].classList.contains('hide')).toEqual(false); + }); + }); + + describe('with a multiple secrets', () => { + const secrets = ['mysecret123', 'happygoat456', 'tanuki789']; + + it('should have correct "Reveal" label when values are hidden', () => { + const wrapper = setupSecretFixture(secrets, false); + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + + expect(revealButton.textContent).toEqual('Reveal values'); + }); + + it('should have correct "Hide" label when values are shown', () => { + const wrapper = setupSecretFixture(secrets, true); + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + + expect(revealButton.textContent).toEqual('Hide values'); + }); + + it('should have all values hidden initially', () => { + const wrapper = setupSecretFixture(secrets, false); + const values = wrapper.querySelectorAll('.js-secret-value'); + const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder'); + + expect(values.length).toEqual(3); + values.forEach((value) => { + expect(value.classList.contains('hide')).toEqual(true); + }); + expect(placeholders.length).toEqual(3); + placeholders.forEach((placeholder) => { + expect(placeholder.classList.contains('hide')).toEqual(false); + }); + }); + + it('should toggle values and placeholders', () => { + const wrapper = setupSecretFixture(secrets, false); + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + const values = wrapper.querySelectorAll('.js-secret-value'); + const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder'); + + revealButton.click(); + + expect(values.length).toEqual(3); + values.forEach((value) => { + expect(value.classList.contains('hide')).toEqual(false); + }); + expect(placeholders.length).toEqual(3); + placeholders.forEach((placeholder) => { + expect(placeholder.classList.contains('hide')).toEqual(true); + }); + + revealButton.click(); + + expect(values.length).toEqual(3); + values.forEach((value) => { + expect(value.classList.contains('hide')).toEqual(true); + }); + expect(placeholders.length).toEqual(3); + placeholders.forEach((placeholder) => { + expect(placeholder.classList.contains('hide')).toEqual(false); + }); + }); + }); +}); diff --git a/spec/javascripts/commits_spec.js b/spec/javascripts/commits_spec.js index e5a5e3293b9..d0176520440 100644 --- a/spec/javascripts/commits_spec.js +++ b/spec/javascripts/commits_spec.js @@ -1,5 +1,4 @@ import 'vendor/jquery.endless-scroll'; -import '~/pager'; import CommitsList from '~/commits'; describe('Commits List', () => { diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 5076435e7a8..31426ceb110 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -2,7 +2,7 @@ /* global Notes */ import * as urlUtils from '~/lib/utils/url_utility'; -import '~/merge_request_tabs'; +import MergeRequestTabs from '~/merge_request_tabs'; import '~/commit/pipelines/pipelines_bundle'; import '~/breakpoints'; import '~/lib/utils/common_utils'; @@ -32,7 +32,7 @@ import 'vendor/jquery.scrollTo'; ); beforeEach(function () { - this.class = new gl.MergeRequestTabs({ stubLocation: stubLocation }); + this.class = new MergeRequestTabs({ stubLocation: stubLocation }); setLocation(); this.spies = { diff --git a/spec/javascripts/pager_spec.js b/spec/javascripts/pager_spec.js index fe3ea996eac..2fd87754238 100644 --- a/spec/javascripts/pager_spec.js +++ b/spec/javascripts/pager_spec.js @@ -1,15 +1,9 @@ /* global fixture */ import * as utils from '~/lib/utils/url_utility'; -import '~/pager'; +import Pager from '~/pager'; describe('pager', () => { - const Pager = window.Pager; - - it('is defined on window', () => { - expect(window.Pager).toBeDefined(); - }); - describe('init', () => { const originalHref = window.location.href; diff --git a/spec/javascripts/vue_shared/components/toggle_button_spec.js b/spec/javascripts/vue_shared/components/toggle_button_spec.js index 447d74d4e08..859995d33fa 100644 --- a/spec/javascripts/vue_shared/components/toggle_button_spec.js +++ b/spec/javascripts/vue_shared/components/toggle_button_spec.js @@ -30,9 +30,9 @@ describe('Toggle Button', () => { expect(vm.$el.querySelector('input').getAttribute('value')).toEqual('true'); }); - it('renders Enabled and Disabled text data attributes', () => { - expect(vm.$el.querySelector('button').getAttribute('data-enabled-text')).toEqual('Enabled'); - expect(vm.$el.querySelector('button').getAttribute('data-disabled-text')).toEqual('Disabled'); + it('renders input status icon', () => { + expect(vm.$el.querySelectorAll('span.toggle-icon').length).toEqual(1); + expect(vm.$el.querySelectorAll('svg.s16.toggle-icon-svg').length).toEqual(1); }); }); @@ -49,6 +49,14 @@ describe('Toggle Button', () => { expect(vm.$el.querySelector('button').classList.contains('is-checked')).toEqual(true); }); + it('sets aria-label representing toggle state', () => { + vm.value = true; + expect(vm.ariaLabel).toEqual('Toggle Status: ON'); + + vm.value = false; + expect(vm.ariaLabel).toEqual('Toggle Status: OFF'); + }); + it('emits change event when clicked', () => { vm.$el.querySelector('button').click(); diff --git a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb index 3998ca940a4..7351d45336a 100644 --- a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb @@ -228,7 +228,7 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migrati let(:projects) { table(:projects) } let(:project) { projects.create(namespace_id: namespace.id, creator_id: author.id) } - # We can not rely on FactoryGirl as the state of Event may change in ways that + # We can not rely on FactoryBot as the state of Event may change in ways that # the background migration does not expect, hence we use the Event class of # the migration itself. def create_push_event(project, author, data = nil) diff --git a/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb index 0560c47f03f..3fe0493ed9b 100644 --- a/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb @@ -23,6 +23,8 @@ describe Gitlab::CycleAnalytics::BaseEventFetcher do allow_any_instance_of(described_class).to receive(:serialize) do |event| event end + allow_any_instance_of(described_class) + .to receive(:allowed_ids).and_return(nil) stub_const('Gitlab::CycleAnalytics::BaseEventFetcher::MAX_EVENTS', max_events) diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb new file mode 100644 index 00000000000..18906955df6 --- /dev/null +++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb @@ -0,0 +1,309 @@ +require 'spec_helper' + +describe Gitlab::Git::GitlabProjects do + after do + TestEnv.clean_test_path + end + + let(:project) { create(:project, :repository) } + + if $VERBOSE + let(:logger) { Logger.new(STDOUT) } + else + let(:logger) { double('logger').as_null_object } + end + + let(:tmp_repos_path) { TestEnv.repos_path } + let(:repo_name) { project.disk_path + '.git' } + let(:tmp_repo_path) { File.join(tmp_repos_path, repo_name) } + let(:gl_projects) { build_gitlab_projects(tmp_repos_path, repo_name) } + + describe '#initialize' do + it { expect(gl_projects.shard_path).to eq(tmp_repos_path) } + it { expect(gl_projects.repository_relative_path).to eq(repo_name) } + it { expect(gl_projects.repository_absolute_path).to eq(File.join(tmp_repos_path, repo_name)) } + it { expect(gl_projects.logger).to eq(logger) } + end + + describe '#mv_project' do + let(:new_repo_path) { File.join(tmp_repos_path, 'repo.git') } + + it 'moves a repo directory' do + expect(File.exist?(tmp_repo_path)).to be_truthy + + message = "Moving repository from <#{tmp_repo_path}> to <#{new_repo_path}>." + expect(logger).to receive(:info).with(message) + + expect(gl_projects.mv_project('repo.git')).to be_truthy + + expect(File.exist?(tmp_repo_path)).to be_falsy + expect(File.exist?(new_repo_path)).to be_truthy + end + + it "fails if the source path doesn't exist" do + expect(logger).to receive(:error).with("mv-project failed: source path <#{tmp_repos_path}/bad-src.git> does not exist.") + + result = build_gitlab_projects(tmp_repos_path, 'bad-src.git').mv_project('repo.git') + expect(result).to be_falsy + end + + it 'fails if the destination path already exists' do + FileUtils.mkdir_p(File.join(tmp_repos_path, 'already-exists.git')) + + message = "mv-project failed: destination path <#{tmp_repos_path}/already-exists.git> already exists." + expect(logger).to receive(:error).with(message) + + expect(gl_projects.mv_project('already-exists.git')).to be_falsy + end + end + + describe '#rm_project' do + it 'removes a repo directory' do + expect(File.exist?(tmp_repo_path)).to be_truthy + expect(logger).to receive(:info).with("Removing repository <#{tmp_repo_path}>.") + + expect(gl_projects.rm_project).to be_truthy + + expect(File.exist?(tmp_repo_path)).to be_falsy + end + end + + describe '#push_branches' do + let(:remote_name) { 'remote-name' } + let(:branch_name) { 'master' } + let(:cmd) { %W(git push -- #{remote_name} #{branch_name}) } + let(:force) { false } + + subject { gl_projects.push_branches(remote_name, 600, force, [branch_name]) } + + it 'executes the command' do + stub_spawn(cmd, 600, tmp_repo_path, success: true) + + is_expected.to be_truthy + end + + it 'fails' do + stub_spawn(cmd, 600, tmp_repo_path, success: false) + + is_expected.to be_falsy + end + + context 'with --force' do + let(:cmd) { %W(git push --force -- #{remote_name} #{branch_name}) } + let(:force) { true } + + it 'executes the command' do + stub_spawn(cmd, 600, tmp_repo_path, success: true) + + is_expected.to be_truthy + end + end + end + + describe '#fetch_remote' do + let(:remote_name) { 'remote-name' } + let(:branch_name) { 'master' } + let(:force) { false } + let(:tags) { true } + let(:args) { { force: force, tags: tags }.merge(extra_args) } + let(:extra_args) { {} } + let(:cmd) { %W(git fetch #{remote_name} --prune --quiet --tags) } + + subject { gl_projects.fetch_remote(remote_name, 600, args) } + + def stub_tempfile(name, filename, opts = {}) + chmod = opts.delete(:chmod) + file = StringIO.new + + allow(file).to receive(:close!) + allow(file).to receive(:path).and_return(name) + + expect(Tempfile).to receive(:new).with(filename).and_return(file) + expect(file).to receive(:chmod).with(chmod) if chmod + + file + end + + context 'with default args' do + it 'executes the command' do + stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) + + is_expected.to be_truthy + end + + it 'fails' do + stub_spawn(cmd, 600, tmp_repo_path, {}, success: false) + + is_expected.to be_falsy + end + end + + context 'with --force' do + let(:force) { true } + let(:cmd) { %W(git fetch #{remote_name} --prune --quiet --force --tags) } + + it 'executes the command with forced option' do + stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) + + is_expected.to be_truthy + end + end + + context 'with --no-tags' do + let(:tags) { false } + let(:cmd) { %W(git fetch #{remote_name} --prune --quiet --no-tags) } + + it 'executes the command' do + stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) + + is_expected.to be_truthy + end + end + + describe 'with an SSH key' do + let(:extra_args) { { ssh_key: 'SSH KEY' } } + + it 'sets GIT_SSH to a custom script' do + script = stub_tempfile('scriptFile', 'gitlab-shell-ssh-wrapper', chmod: 0o755) + key = stub_tempfile('/tmp files/keyFile', 'gitlab-shell-key-file', chmod: 0o400) + + stub_spawn(cmd, 600, tmp_repo_path, { 'GIT_SSH' => 'scriptFile' }, success: true) + + is_expected.to be_truthy + + expect(script.string).to eq("#!/bin/sh\nexec ssh '-oIdentityFile=\"/tmp files/keyFile\"' '-oIdentitiesOnly=\"yes\"' \"$@\"") + expect(key.string).to eq('SSH KEY') + end + end + + describe 'with known_hosts data' do + let(:extra_args) { { known_hosts: 'KNOWN HOSTS' } } + + it 'sets GIT_SSH to a custom script' do + script = stub_tempfile('scriptFile', 'gitlab-shell-ssh-wrapper', chmod: 0o755) + key = stub_tempfile('/tmp files/knownHosts', 'gitlab-shell-known-hosts', chmod: 0o400) + + stub_spawn(cmd, 600, tmp_repo_path, { 'GIT_SSH' => 'scriptFile' }, success: true) + + is_expected.to be_truthy + + expect(script.string).to eq("#!/bin/sh\nexec ssh '-oStrictHostKeyChecking=\"yes\"' '-oUserKnownHostsFile=\"/tmp files/knownHosts\"' \"$@\"") + expect(key.string).to eq('KNOWN HOSTS') + end + end + end + + describe '#import_project' do + let(:project) { create(:project) } + let(:import_url) { TestEnv.factory_repo_path_bare } + let(:cmd) { %W(git clone --bare -- #{import_url} #{tmp_repo_path}) } + let(:timeout) { 600 } + + subject { gl_projects.import_project(import_url, timeout) } + + context 'success import' do + it 'imports a repo' do + expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy + + message = "Importing project from <#{import_url}> to <#{tmp_repo_path}>." + expect(logger).to receive(:info).with(message) + + is_expected.to be_truthy + + expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_truthy + end + end + + context 'already exists' do + it "doesn't import" do + FileUtils.mkdir_p(tmp_repo_path) + + is_expected.to be_falsy + end + end + + context 'timeout' do + it 'does not import a repo' do + stub_spawn_timeout(cmd, timeout, nil) + + message = "Importing project from <#{import_url}> to <#{tmp_repo_path}> failed." + expect(logger).to receive(:error).with(message) + + is_expected.to be_falsy + + expect(gl_projects.output).to eq("Timed out\n") + expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy + end + end + end + + describe '#fork_repository' do + let(:dest_repos_path) { tmp_repos_path } + let(:dest_repo_name) { File.join('@hashed', 'aa', 'bb', 'xyz.git') } + let(:dest_repo) { File.join(dest_repos_path, dest_repo_name) } + let(:dest_namespace) { File.dirname(dest_repo) } + + subject { gl_projects.fork_repository(dest_repos_path, dest_repo_name) } + + before do + FileUtils.mkdir_p(dest_repos_path) + end + + after do + FileUtils.rm_rf(dest_repos_path) + end + + it 'forks the repository' do + message = "Forking repository from <#{tmp_repo_path}> to <#{dest_repo}>." + expect(logger).to receive(:info).with(message) + + is_expected.to be_truthy + + expect(File.exist?(dest_repo)).to be_truthy + expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy + expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy + end + + it 'does not fork if a project of the same name already exists' do + # create a fake project at the intended destination + FileUtils.mkdir_p(dest_repo) + + # trying to fork again should fail as the repo already exists + message = "fork-repository failed: destination repository <#{dest_repo}> already exists." + expect(logger).to receive(:error).with(message) + + is_expected.to be_falsy + end + + context 'different storages' do + let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), 'alternative') } + + it 'forks the repo' do + is_expected.to be_truthy + + expect(File.exist?(dest_repo)).to be_truthy + expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy + expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy + end + end + end + + def build_gitlab_projects(*args) + described_class.new( + *args, + global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, + logger: logger + ) + end + + def stub_spawn(*args, success: true) + exitstatus = success ? 0 : nil + expect(gl_projects).to receive(:popen_with_timeout).with(*args) + .and_return(["output", exitstatus]) + end + + def stub_spawn_timeout(*args) + expect(gl_projects).to receive(:popen_with_timeout).with(*args) + .and_raise(Timeout::Error) + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index e6845420f7d..03a9cc488ca 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -19,6 +19,51 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + describe '.create_hooks' do + let(:repo_path) { File.join(TestEnv.repos_path, 'hook-test.git') } + let(:hooks_dir) { File.join(repo_path, 'hooks') } + let(:target_hooks_dir) { Gitlab.config.gitlab_shell.hooks_path } + let(:existing_target) { File.join(repo_path, 'foobar') } + + before do + FileUtils.rm_rf(repo_path) + FileUtils.mkdir_p(repo_path) + end + + context 'hooks is a directory' do + let(:existing_file) { File.join(hooks_dir, 'my-file') } + + before do + FileUtils.mkdir_p(hooks_dir) + FileUtils.touch(existing_file) + described_class.create_hooks(repo_path, target_hooks_dir) + end + + it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } + it { expect(Dir[File.join(repo_path, "hooks.old.*/my-file")].count).to eq(1) } + end + + context 'hooks is a valid symlink' do + before do + FileUtils.mkdir_p existing_target + File.symlink(existing_target, hooks_dir) + described_class.create_hooks(repo_path, target_hooks_dir) + end + + it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } + end + + context 'hooks is a broken symlink' do + before do + FileUtils.rm_f(existing_target) + File.symlink(existing_target, hooks_dir) + described_class.create_hooks(repo_path, target_hooks_dir) + end + + it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } + end + end + describe "Respond to" do subject { repository } diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 260df6e4dae..048caa38fcf 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -38,7 +38,6 @@ describe Gitlab::LDAP::User do it "does not mark existing ldap user as changed" do create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain') - ldap_user.gl_user.user_synced_attributes_metadata(provider: 'ldapmain', email: true) expect(ldap_user.changed?).to be_falsey end end @@ -144,11 +143,15 @@ describe Gitlab::LDAP::User do expect(ldap_user.gl_user.email).to eq(info[:email]) end - it "has user_synced_attributes_metadata email set to true" do + it "has email set as synced" do expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_truthy end - it "has synced_attribute_provider set to ldapmain" do + it "has email set as read-only" do + expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_truthy + end + + it "has synced attributes provider set to ldapmain" do expect(ldap_user.gl_user.user_synced_attributes_metadata.provider).to eql 'ldapmain' end end @@ -162,9 +165,13 @@ describe Gitlab::LDAP::User do expect(ldap_user.gl_user.temp_oauth_email?).to be_truthy end - it "has synced attribute email set to false" do + it "has email set as not synced" do expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_falsey end + + it "does not have email set as read-only" do + expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_falsey + end end end diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 2f19fb7312d..6334bcd0156 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -202,11 +202,13 @@ describe Gitlab::OAuth::User do end context "and no account for the LDAP user" do - it "creates a user with dual LDAP and omniauth identities" do + before do allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) oauth_user.save + end + it "creates a user with dual LDAP and omniauth identities" do expect(gl_user).to be_valid expect(gl_user.username).to eql uid expect(gl_user.email).to eql 'johndoe@example.com' @@ -219,6 +221,18 @@ describe Gitlab::OAuth::User do ] ) end + + it "has email set as synced" do + expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy + end + + it "has email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_truthy + end + + it "has synced attributes provider set to ldapmain" do + expect(gl_user.user_synced_attributes_metadata.provider).to eql 'ldapmain' + end end context "and LDAP user has an account already" do @@ -440,11 +454,15 @@ describe Gitlab::OAuth::User do expect(gl_user.email).to eq(info_hash[:email]) end - it "has external_attributes set to true" do - expect(gl_user.user_synced_attributes_metadata).not_to be_nil + it "has email set as synced" do + expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy + end + + it "has email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_truthy end - it "has attributes_provider set to my-provider" do + it "has synced attributes provider set to my-provider" do expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider' end end @@ -458,10 +476,13 @@ describe Gitlab::OAuth::User do expect(gl_user.email).not_to eq(info_hash[:email]) end - it "has user_synced_attributes_metadata set to nil" do - expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider' + it "has email set as not synced" do expect(gl_user.user_synced_attributes_metadata.email_synced).to be_falsey end + + it "does not have email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_falsey + end end end @@ -508,11 +529,15 @@ describe Gitlab::OAuth::User do expect(gl_user.email).to eq(info_hash[:email]) end - it "has email_synced_attribute set to true" do + it "has email set as synced" do expect(gl_user.user_synced_attributes_metadata.email_synced).to be(true) end - it "has my-provider as attributes_provider" do + it "has email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_truthy + end + + it "has synced attributes provider set to my-provider" do expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider' end end @@ -524,7 +549,14 @@ describe Gitlab::OAuth::User do it "does not update the user email" do expect(gl_user.email).not_to eq(info_hash[:email]) - expect(gl_user.user_synced_attributes_metadata.email_synced).to be(false) + end + + it "has email set as not synced" do + expect(gl_user.user_synced_attributes_metadata.email_synced).to be_falsey + end + + it "does not have email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_falsey end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index eec6858a5de..dd779b04741 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -2,12 +2,19 @@ require 'spec_helper' require 'stringio' describe Gitlab::Shell do - let(:project) { double('Project', id: 7, path: 'diaspora') } + set(:project) { create(:project, :repository) } + let(:gitlab_shell) { described_class.new } let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } } + let(:gitlab_projects) { double('gitlab_projects') } + let(:timeout) { Gitlab.config.gitlab_shell.git_timeout } before do allow(Project).to receive(:find).and_return(project) + + allow(gitlab_shell).to receive(:gitlab_projects) + .with(project.repository_storage_path, project.disk_path + '.git') + .and_return(gitlab_projects) end it { is_expected.to respond_to :add_key } @@ -44,38 +51,6 @@ describe Gitlab::Shell do end end - describe 'projects commands' do - let(:gitlab_shell_path) { File.expand_path('tmp/tests/gitlab-shell') } - let(:projects_path) { File.join(gitlab_shell_path, 'bin/gitlab-projects') } - let(:gitlab_shell_hooks_path) { File.join(gitlab_shell_path, 'hooks') } - - before do - allow(Gitlab.config.gitlab_shell).to receive(:path).and_return(gitlab_shell_path) - allow(Gitlab.config.gitlab_shell).to receive(:hooks_path).and_return(gitlab_shell_hooks_path) - allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800) - end - - describe '#mv_repository' do - it 'executes the command' do - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [projects_path, 'mv-project', 'storage/path', 'project/path.git', 'new/path.git'] - ) - gitlab_shell.mv_repository('storage/path', 'project/path', 'new/path') - end - end - - describe '#add_key' do - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( - [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] - ) - - gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') - end - end - end - describe Gitlab::Shell::KeyAdder do describe '#add_key' do it 'removes trailing garbage' do @@ -121,6 +96,17 @@ describe Gitlab::Shell do allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800) end + describe '#add_key' do + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with( + [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] + ) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + describe '#add_repository' do shared_examples '#add_repository' do let(:repository_storage) { 'default' } @@ -162,83 +148,76 @@ describe Gitlab::Shell do end describe '#remove_repository' do + subject { gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path) } + it 'returns true when the command succeeds' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'rm-project', 'current/storage', 'project/path.git'], - nil, popen_vars).and_return([nil, 0]) + expect(gitlab_projects).to receive(:rm_project) { true } - expect(gitlab_shell.remove_repository('current/storage', 'project/path')).to be true + is_expected.to be_truthy end it 'returns false when the command fails' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'rm-project', 'current/storage', 'project/path.git'], - nil, popen_vars).and_return(["error", 1]) + expect(gitlab_projects).to receive(:rm_project) { false } - expect(gitlab_shell.remove_repository('current/storage', 'project/path')).to be false + is_expected.to be_falsy end end describe '#mv_repository' do it 'returns true when the command succeeds' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'mv-project', 'current/storage', 'project/path.git', 'project/newpath.git'], - nil, popen_vars).and_return([nil, 0]) + expect(gitlab_projects).to receive(:mv_project).with('project/newpath.git') { true } - expect(gitlab_shell.mv_repository('current/storage', 'project/path', 'project/newpath')).to be true + expect(gitlab_shell.mv_repository(project.repository_storage_path, project.disk_path, 'project/newpath')).to be_truthy end it 'returns false when the command fails' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'mv-project', 'current/storage', 'project/path.git', 'project/newpath.git'], - nil, popen_vars).and_return(["error", 1]) + expect(gitlab_projects).to receive(:mv_project).with('project/newpath.git') { false } - expect(gitlab_shell.mv_repository('current/storage', 'project/path', 'project/newpath')).to be false + expect(gitlab_shell.mv_repository(project.repository_storage_path, project.disk_path, 'project/newpath')).to be_falsy end end describe '#fork_repository' do + subject do + gitlab_shell.fork_repository( + project.repository_storage_path, + project.disk_path, + 'new/storage', + 'fork/path' + ) + end + it 'returns true when the command succeeds' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'fork-repository', 'current/storage', 'project/path.git', 'new/storage', 'fork/path.git'], - nil, popen_vars).and_return([nil, 0]) + expect(gitlab_projects).to receive(:fork_repository).with('new/storage', 'fork/path.git') { true } - expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'fork/path')).to be true + is_expected.to be_truthy end it 'return false when the command fails' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'fork-repository', 'current/storage', 'project/path.git', 'new/storage', 'fork/path.git'], - nil, popen_vars).and_return(["error", 1]) + expect(gitlab_projects).to receive(:fork_repository).with('new/storage', 'fork/path.git') { false } - expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'fork/path')).to be false + is_expected.to be_falsy end end shared_examples 'fetch_remote' do |gitaly_on| - let(:project2) { create(:project, :repository) } - let(:repository) { project2.repository } + let(:repository) { project.repository } def fetch_remote(ssh_auth = nil) - gitlab_shell.fetch_remote(repository.raw_repository, 'new/storage', ssh_auth: ssh_auth) + gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', ssh_auth: ssh_auth) end - def expect_popen(fail = false, vars = {}) - popen_args = [ - projects_path, - 'fetch-remote', - TestEnv.repos_path, - repository.relative_path, - 'new/storage', - Gitlab.config.gitlab_shell.git_timeout.to_s - ] - - return_value = fail ? ["error", 1] : [nil, 0] + def expect_gitlab_projects(fail = false, options = {}) + expect(gitlab_projects).to receive(:fetch_remote).with( + 'remote-name', + timeout, + options + ).and_return(!fail) - expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)).and_return(return_value) + allow(gitlab_projects).to receive(:output).and_return('error') if fail end - def expect_gitaly_call(fail, vars = {}) + def expect_gitaly_call(fail, options = {}) receive_fetch_remote = if fail receive(:fetch_remote).and_raise(GRPC::NotFound) @@ -250,12 +229,12 @@ describe Gitlab::Shell do end if gitaly_on - def expect_call(fail, vars = {}) - expect_gitaly_call(fail, vars) + def expect_call(fail, options = {}) + expect_gitaly_call(fail, options) end else - def expect_call(fail, vars = {}) - expect_popen(fail, vars) + def expect_call(fail, options = {}) + expect_gitlab_projects(fail, options) end end @@ -271,20 +250,27 @@ describe Gitlab::Shell do end it 'returns true when the command succeeds' do - expect_call(false) + expect_call(false, force: false, tags: true) expect(fetch_remote).to be_truthy end it 'raises an exception when the command fails' do - expect_call(true) + expect_call(true, force: false, tags: true) expect { fetch_remote }.to raise_error(Gitlab::Shell::Error) end + it 'allows forced and no_tags to be changed' do + expect_call(false, force: true, tags: false) + + result = gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', forced: true, no_tags: true) + expect(result).to be_truthy + end + context 'SSH auth' do it 'passes the SSH key if specified' do - expect_call(false, 'GITLAB_SHELL_SSH_KEY' => 'foo') + expect_call(false, force: false, tags: true, ssh_key: 'foo') ssh_auth = build_ssh_auth(ssh_key_auth?: true, ssh_private_key: 'foo') @@ -292,7 +278,7 @@ describe Gitlab::Shell do end it 'does not pass an empty SSH key' do - expect_call(false) + expect_call(false, force: false, tags: true) ssh_auth = build_ssh_auth(ssh_key_auth: true, ssh_private_key: '') @@ -300,7 +286,7 @@ describe Gitlab::Shell do end it 'does not pass the key unless SSH key auth is to be used' do - expect_call(false) + expect_call(false, force: false, tags: true) ssh_auth = build_ssh_auth(ssh_key_auth: false, ssh_private_key: 'foo') @@ -308,7 +294,7 @@ describe Gitlab::Shell do end it 'passes the known_hosts data if specified' do - expect_call(false, 'GITLAB_SHELL_KNOWN_HOSTS' => 'foo') + expect_call(false, force: false, tags: true, known_hosts: 'foo') ssh_auth = build_ssh_auth(ssh_known_hosts: 'foo') @@ -316,7 +302,7 @@ describe Gitlab::Shell do end it 'does not pass empty known_hosts data' do - expect_call(false) + expect_call(false, force: false, tags: true) ssh_auth = build_ssh_auth(ssh_known_hosts: '') @@ -324,7 +310,7 @@ describe Gitlab::Shell do end it 'does not pass known_hosts data unless SSH is to be used' do - expect_call(false, popen_vars) + expect_call(false, force: false, tags: true) ssh_auth = build_ssh_auth(ssh_import?: false, ssh_known_hosts: 'foo') @@ -342,20 +328,79 @@ describe Gitlab::Shell do end describe '#import_repository' do + let(:import_url) { 'https://gitlab.com/gitlab-org/gitlab-ce.git' } + it 'returns true when the command succeeds' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"], - nil, popen_vars).and_return([nil, 0]) + expect(gitlab_projects).to receive(:import_project).with(import_url, timeout) { true } - expect(gitlab_shell.import_repository('current/storage', 'project/path', 'https://gitlab.com/gitlab-org/gitlab-ce.git')).to be true + result = gitlab_shell.import_repository(project.repository_storage_path, project.disk_path, import_url) + + expect(result).to be_truthy end it 'raises an exception when the command fails' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"], - nil, popen_vars).and_return(["error", 1]) + allow(gitlab_projects).to receive(:output) { 'error' } + expect(gitlab_projects).to receive(:import_project) { false } + + expect do + gitlab_shell.import_repository(project.repository_storage_path, project.disk_path, import_url) + end.to raise_error(Gitlab::Shell::Error, "error") + end + end + + describe '#push_remote_branches' do + subject(:result) do + gitlab_shell.push_remote_branches( + project.repository_storage_path, + project.disk_path, + 'downstream-remote', + ['master'] + ) + end + + it 'executes the command' do + expect(gitlab_projects).to receive(:push_branches) + .with('downstream-remote', timeout, true, ['master']) + .and_return(true) + + is_expected.to be_truthy + end + + it 'fails to execute the command' do + allow(gitlab_projects).to receive(:output) { 'error' } + expect(gitlab_projects).to receive(:push_branches) + .with('downstream-remote', timeout, true, ['master']) + .and_return(false) + + expect { result }.to raise_error(Gitlab::Shell::Error, 'error') + end + end + + describe '#delete_remote_branches' do + subject(:result) do + gitlab_shell.delete_remote_branches( + project.repository_storage_path, + project.disk_path, + 'downstream-remote', + ['master'] + ) + end + + it 'executes the command' do + expect(gitlab_projects).to receive(:delete_remote_branches) + .with('downstream-remote', ['master']) + .and_return(true) + + is_expected.to be_truthy + end + + it 'fails to execute the command' do + allow(gitlab_projects).to receive(:output) { 'error' } + expect(gitlab_projects).to receive(:delete_remote_branches) + .with('downstream-remote', ['master']) + .and_return(false) - expect { gitlab_shell.import_repository('current/storage', 'project/path', 'https://gitlab.com/gitlab-org/gitlab-ce.git') }.to raise_error(Gitlab::Shell::Error, "error") + expect { result }.to raise_error(Gitlab::Shell::Error, 'error') end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index bb89e093890..856e17b20bd 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -116,7 +116,7 @@ describe Ci::Pipeline, :mailer do end it "calculates average when there is one build without coverage" do - FactoryGirl.create(:ci_build, pipeline: pipeline) + FactoryBot.create(:ci_build, pipeline: pipeline) expect(pipeline.coverage).to be_nil end end @@ -435,7 +435,7 @@ describe Ci::Pipeline, :mailer do describe 'merge request metrics' do let(:project) { create(:project, :repository) } - let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } + let(:pipeline) { FactoryBot.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } before do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index a93e7e233a8..b2b64e6ff48 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -51,24 +51,24 @@ describe Ci::Runner do describe '#display_name' do it 'returns the description if it has a value' do - runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') + runner = FactoryBot.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') expect(runner.display_name).to eq 'Linux/Ruby-1.9.3-p448' end it 'returns the token if it does not have a description' do - runner = FactoryGirl.create(:ci_runner) + runner = FactoryBot.create(:ci_runner) expect(runner.display_name).to eq runner.description end it 'returns the token if the description is an empty string' do - runner = FactoryGirl.build(:ci_runner, description: '', token: 'token') + runner = FactoryBot.build(:ci_runner, description: '', token: 'token') expect(runner.display_name).to eq runner.token end end describe '#assign_to' do - let!(:project) { FactoryGirl.create :project } - let!(:shared_runner) { FactoryGirl.create(:ci_runner, :shared) } + let!(:project) { FactoryBot.create :project } + let!(:shared_runner) { FactoryBot.create(:ci_runner, :shared) } before do shared_runner.assign_to(project) @@ -83,15 +83,15 @@ describe Ci::Runner do subject { described_class.online } before do - @runner1 = FactoryGirl.create(:ci_runner, :shared, contacted_at: 1.year.ago) - @runner2 = FactoryGirl.create(:ci_runner, :shared, contacted_at: 1.second.ago) + @runner1 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.year.ago) + @runner2 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) end it { is_expected.to eq([@runner2])} end describe '#online?' do - let(:runner) { FactoryGirl.create(:ci_runner, :shared) } + let(:runner) { FactoryBot.create(:ci_runner, :shared) } subject { runner.online? } @@ -268,7 +268,7 @@ describe Ci::Runner do end describe '#status' do - let(:runner) { FactoryGirl.create(:ci_runner, :shared, contacted_at: 1.second.ago) } + let(:runner) { FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) } subject { runner.status } @@ -442,9 +442,9 @@ describe Ci::Runner do describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do - runner = FactoryGirl.create(:ci_runner) - project = FactoryGirl.create(:project) - project1 = FactoryGirl.create(:project) + runner = FactoryBot.create(:ci_runner) + project = FactoryBot.create(:project) + project1 = FactoryBot.create(:project) project.runners << runner project1.runners << runner @@ -452,8 +452,8 @@ describe Ci::Runner do end it "returns true" do - runner = FactoryGirl.create(:ci_runner) - project = FactoryGirl.create(:project) + runner = FactoryBot.create(:ci_runner) + project = FactoryBot.create(:project) project.runners << runner expect(runner.belongs_to_one_project?).to be_truthy diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index c5708e70ef9..ba8aa13d5ad 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -126,7 +126,7 @@ describe Deployment do subject { deployment.stop_action } context 'when no other actions' do - let(:deployment) { FactoryGirl.build(:deployment, deployable: build) } + let(:deployment) { FactoryBot.build(:deployment, deployable: build) } it { is_expected.to be_nil } end @@ -135,13 +135,13 @@ describe Deployment do let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') } context 'when matching action is defined' do - let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_other_app') } + let(:deployment) { FactoryBot.build(:deployment, deployable: build, on_stop: 'close_other_app') } it { is_expected.to be_nil } end context 'when no matching action is defined' do - let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_app') } + let(:deployment) { FactoryBot.build(:deployment, deployable: build, on_stop: 'close_app') } it { is_expected.to eq(close_action) } end @@ -159,7 +159,7 @@ describe Deployment do context 'when matching action is defined' do let(:build) { create(:ci_build) } - let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_app') } + let(:deployment) { FactoryBot.build(:deployment, deployable: build, on_stop: 'close_app') } let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') } it { is_expected.to be_truthy } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dd9e8498519..f805f2dcddb 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -289,12 +289,12 @@ describe Project do describe 'project token' do it 'sets an random token if none provided' do - project = FactoryGirl.create :project, runners_token: '' + project = FactoryBot.create :project, runners_token: '' expect(project.runners_token).not_to eq('') end it 'does not set an random token if one provided' do - project = FactoryGirl.create :project, runners_token: 'my-token' + project = FactoryBot.create :project, runners_token: 'my-token' expect(project.runners_token).to eq('my-token') end end diff --git a/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb b/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb new file mode 100644 index 00000000000..1fd40653f79 --- /dev/null +++ b/spec/rubocop/cop/gitlab/module_with_instance_variables_spec.rb @@ -0,0 +1,157 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/module_with_instance_variables' + +describe RuboCop::Cop::Gitlab::ModuleWithInstanceVariables 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 instance variable is used in a module' do + inspect_source(cop, 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(cop, source) + + expect(cop.offenses).to be_empty + end + end + + context 'when source is a regular module' do + it_behaves_like 'registering offense', offending_lines: [3] do + let(:source) do + <<~RUBY + module M + def f + @f = true + end + end + RUBY + end + end + end + + context 'when source is a nested module' do + it_behaves_like 'registering offense', offending_lines: [4] do + let(:source) do + <<~RUBY + module N + module M + def f + @f = true + end + end + end + RUBY + end + end + end + + context 'when source is a nested module with multiple offenses' do + it_behaves_like 'registering offense', offending_lines: [4, 12] do + let(:source) do + <<~RUBY + module N + module M + def f + @f = true + end + + def g + true + end + + def h + @h = true + end + end + end + RUBY + end + end + end + + context 'when source is using simple or ivar assignment' do + it_behaves_like 'not registering offense' do + let(:source) do + <<~RUBY + module M + def f + @f ||= true + end + end + RUBY + end + end + end + + context 'when source is using simple ivar' do + it_behaves_like 'not registering offense' do + let(:source) do + <<~RUBY + module M + def f? + @f + end + end + RUBY + end + end + end + + context 'when source is defining initialize' do + it_behaves_like 'not registering offense' do + let(:source) do + <<~RUBY + module M + def initialize + @a = 1 + @b = 2 + end + end + RUBY + end + end + end + + context 'when source is using simple or ivar assignment with other ivar' do + it_behaves_like 'registering offense', offending_lines: [3] do + let(:source) do + <<~RUBY + module M + def f + @f ||= g(@g) + end + end + RUBY + end + end + end + + context 'when source is using or ivar assignment with something else' do + it_behaves_like 'registering offense', offending_lines: [3, 4] do + let(:source) do + <<~RUBY + module M + def f + @f ||= true + @f.to_s + end + end + RUBY + end + end + end +end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index de8a9ce12ff..97a563c1ce1 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' module Ci describe RegisterJobService do - let!(:project) { FactoryGirl.create :project, shared_runners_enabled: false } - let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } - let!(:pending_job) { FactoryGirl.create :ci_build, pipeline: pipeline } - let!(:shared_runner) { FactoryGirl.create(:ci_runner, is_shared: true) } - let!(:specific_runner) { FactoryGirl.create(:ci_runner, is_shared: false) } + let!(:project) { FactoryBot.create :project, shared_runners_enabled: false } + let!(:pipeline) { FactoryBot.create :ci_pipeline, project: project } + let!(:pending_job) { FactoryBot.create :ci_build, pipeline: pipeline } + let!(:shared_runner) { FactoryBot.create(:ci_runner, is_shared: true) } + let!(:specific_runner) { FactoryBot.create(:ci_runner, is_shared: false) } before do specific_runner.assign_to(project) @@ -74,11 +74,11 @@ module Ci let!(:project3) { create :project, shared_runners_enabled: true } let!(:pipeline3) { create :ci_pipeline, project: project3 } let!(:build1_project1) { pending_job } - let!(:build2_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } - let!(:build3_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } - let!(:build1_project2) { FactoryGirl.create :ci_build, pipeline: pipeline2 } - let!(:build2_project2) { FactoryGirl.create :ci_build, pipeline: pipeline2 } - let!(:build1_project3) { FactoryGirl.create :ci_build, pipeline: pipeline3 } + let!(:build2_project1) { FactoryBot.create :ci_build, pipeline: pipeline } + let!(:build3_project1) { FactoryBot.create :ci_build, pipeline: pipeline } + let!(:build1_project2) { FactoryBot.create :ci_build, pipeline: pipeline2 } + let!(:build2_project2) { FactoryBot.create :ci_build, pipeline: pipeline2 } + let!(:build1_project3) { FactoryBot.create :ci_build, pipeline: pipeline3 } it 'prefers projects without builds first' do # it gets for one build from each of the projects diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index d48a44fa57f..a06397a0782 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -40,7 +40,7 @@ describe Ci::RetryBuildService do description: 'my-job', stage: 'test', pipeline: pipeline, auto_canceled_by: create(:ci_empty_pipeline, project: project)) do |build| ## - # TODO, workaround for FactoryGirl limitation when having both + # TODO, workaround for FactoryBot limitation when having both # stage (text) and stage_id (integer) columns in the table. build.stage_id = stage.id end diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb index 5376083e7f5..e28d8d7ae5c 100644 --- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb +++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb @@ -213,7 +213,7 @@ describe MergeRequests::Conflicts::ResolveService do MergeRequests::Conflicts::ListService.new(merge_request).conflicts.resolver end let(:regex_conflict) do - resolver.conflict_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb') + resolver.conflict_for_path(resolver.conflicts, 'files/ruby/regex.rb', 'files/ruby/regex.rb') end let(:invalid_params) do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f94fb8733d5..f51bb44086b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -183,7 +183,7 @@ RSpec::Matchers.define :match_asset_path do |expected| end end -FactoryGirl::SyntaxRunner.class_eval do +FactoryBot::SyntaxRunner.class_eval do include RSpec::Mocks::ExampleMethods end diff --git a/spec/support/factory_girl.rb b/spec/support/factory_girl.rb index eec437fb3aa..c7890e49c66 100644 --- a/spec/support/factory_girl.rb +++ b/spec/support/factory_girl.rb @@ -1,3 +1,3 @@ RSpec.configure do |config| - config.include FactoryGirl::Syntax::Methods + config.include FactoryBot::Syntax::Methods end diff --git a/spec/support/markdown_feature.rb b/spec/support/markdown_feature.rb index c90359d7cfa..a0d854d3641 100644 --- a/spec/support/markdown_feature.rb +++ b/spec/support/markdown_feature.rb @@ -8,7 +8,7 @@ # The class renders `spec/fixtures/markdown.md.erb` using ERB, allowing for # reference to the factory-created objects. class MarkdownFeature - include FactoryGirl::Syntax::Methods + include FactoryBot::Syntax::Methods def user @user ||= create(:user) diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb index b36cf3c544c..9f08c139322 100644 --- a/spec/support/stub_configuration.rb +++ b/spec/support/stub_configuration.rb @@ -7,6 +7,9 @@ module StubConfiguration allow_any_instance_of(ApplicationSetting).to receive_messages(to_settings(messages)) allow(Gitlab::CurrentSettings.current_application_settings) .to receive_messages(to_settings(messages)) + + # Ensure that we don't use the Markdown cache when stubbing these values + allow_any_instance_of(ApplicationSetting).to receive(:cached_html_up_to_date?).and_return(false) end def stub_not_protect_default_branch diff --git a/spec/support/stub_env.rb b/spec/support/stub_env.rb index 19fbe572930..f621463e621 100644 --- a/spec/support/stub_env.rb +++ b/spec/support/stub_env.rb @@ -17,6 +17,7 @@ module StubENV def add_stubbed_value(key, value) allow(ENV).to receive(:[]).with(key).and_return(value) + allow(ENV).to receive(:key?).with(key).and_return(true) allow(ENV).to receive(:fetch).with(key).and_return(value) allow(ENV).to receive(:fetch).with(key, anything()) do |_, default_val| value || default_val @@ -29,6 +30,7 @@ module StubENV def init_stub allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:key?).and_call_original allow(ENV).to receive(:fetch).and_call_original add_stubbed_value(STUBBED_KEY, true) end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index b300b493f86..ffc051a3fff 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -82,10 +82,10 @@ module TestEnv setup_gitaly - # Create repository for FactoryGirl.create(:project) + # Create repository for FactoryBot.create(:project) setup_factory_repo - # Create repository for FactoryGirl.create(:forked_project_with_submodules) + # Create repository for FactoryBot.create(:forked_project_with_submodules) setup_forked_repo end diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb index bb32ee62ccb..7ef7fb7d758 100644 --- a/spec/uploaders/records_uploads_spec.rb +++ b/spec/uploaders/records_uploads_spec.rb @@ -8,7 +8,7 @@ describe RecordsUploads do storage :file def model - FactoryGirl.build_stubbed(:user) + FactoryBot.build_stubbed(:user) end end diff --git a/spec/views/projects/jobs/show.html.haml_spec.rb b/spec/views/projects/jobs/show.html.haml_spec.rb index 6139529013f..6a67da79ec5 100644 --- a/spec/views/projects/jobs/show.html.haml_spec.rb +++ b/spec/views/projects/jobs/show.html.haml_spec.rb @@ -187,7 +187,7 @@ describe 'projects/jobs/show' do context 'when incomplete trigger_request is used' do before do - build.trigger_request = FactoryGirl.build(:ci_trigger_request, trigger: nil) + build.trigger_request = FactoryBot.build(:ci_trigger_request, trigger: nil) end it 'test should not render token block' do @@ -199,7 +199,7 @@ describe 'projects/jobs/show' do context 'when complete trigger_request is used' do before do - build.trigger_request = FactoryGirl.build(:ci_trigger_request) + build.trigger_request = FactoryBot.build(:ci_trigger_request) end it 'should render token' do diff --git a/spec/views/projects/tree/_blob_item.html.haml_spec.rb b/spec/views/projects/tree/_blob_item.html.haml_spec.rb new file mode 100644 index 00000000000..6a477c712ff --- /dev/null +++ b/spec/views/projects/tree/_blob_item.html.haml_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe 'projects/tree/_blob_item' do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:blob_item) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID, 'files/ruby').first } + + before do + assign(:project, project) + assign(:repository, repository) + assign(:id, File.join('master', '')) + assign(:lfs_blob_ids, []) + end + + it 'renders blob item' do + render_partial(blob_item) + + expect(rendered).to have_content(blob_item.name) + expect(rendered).not_to have_selector('.label-lfs', text: 'LFS') + end + + describe 'LFS blob' do + before do + assign(:lfs_blob_ids, [blob_item].map(&:id)) + + render_partial(blob_item) + end + + it 'renders LFS badge' do + expect(rendered).to have_selector('.label-lfs', text: 'LFS') + end + end + + def render_partial(blob_item) + render partial: 'projects/tree/blob_item', locals: { + blob_item: blob_item, + type: 'blob' + } + end +end diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb index 3c25e341b39..44b32df0395 100644 --- a/spec/views/projects/tree/show.html.haml_spec.rb +++ b/spec/views/projects/tree/show.html.haml_spec.rb @@ -9,6 +9,7 @@ describe 'projects/tree/show' do before do assign(:project, project) assign(:repository, repository) + assign(:lfs_blob_ids, []) allow(view).to receive(:can?).and_return(true) allow(view).to receive(:can_collaborate_with_project?).and_return(true) diff --git a/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml b/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml index da4d86b9a04..275487071f3 100644 --- a/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml @@ -89,7 +89,7 @@ sast: POSTGRES_DB: "false" allow_failure: true script: - - /app/bin/run . + - sast . artifacts: paths: [gl-sast-report.json] @@ -232,6 +232,17 @@ production: docker run ${cc_opts} codeclimate/codeclimate:0.69.0 analyze -f json > codeclimate.json } + function sast() { + case "$CI_SERVER_VERSION" in + *-ee) + /app/bin/run "$@" + ;; + *) + echo "GitLab EE is required" + ;; + esac + } + function deploy() { track="${1-stable}" name="$CI_ENVIRONMENT_SLUG" diff --git a/yarn.lock b/yarn.lock index 5ff75b161a4..c4d1bd3c682 100644 --- a/yarn.lock +++ b/yarn.lock @@ -54,9 +54,9 @@ lodash "^4.2.0" to-fast-properties "^2.0.0" -"@gitlab-org/gitlab-svgs@^1.2.0": - version "1.2.0" - resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.2.0.tgz#0b1181b5d2dd56a959528529750417c5f49159ca" +"@gitlab-org/gitlab-svgs@^1.3.0": + version "1.3.0" + resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.3.0.tgz#07f2aa75d6e0e857eaa20c38a3bad7e6c22c420c" "@types/jquery@^2.0.40": version "2.0.48" |