diff options
| author | Phil Hughes <me@iamphill.com> | 2017-12-18 10:31:21 +0000 |
|---|---|---|
| committer | Phil Hughes <me@iamphill.com> | 2017-12-18 10:31:21 +0000 |
| commit | 0a05d8740ac710b814a8f89cec3fc85d6fae6be9 (patch) | |
| tree | e28de222d7d7334d6784be7f612e7763f016ae4a | |
| parent | eaf2f48dc7db706fa1dea050543667f8cdebd4c4 (diff) | |
| parent | b4ea25cafa0c7768a8618c673f9a1830f27fc950 (diff) | |
| download | gitlab-ce-0a05d8740ac710b814a8f89cec3fc85d6fae6be9.tar.gz | |
Merge branch 'master' into 'ph-es-notes-module'ph-es-notes-module
# Conflicts:
# app/assets/javascripts/main.js
252 files changed, 1709 insertions, 903 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 19540e4331e..d35fd28c766 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" @@ -76,10 +76,15 @@ stages: except: - /(^docs[\/-].*|.*-docs$)/ +.except-qa: &except-qa + except: + - /(^qa[\/-].*|.*-qa$)/ + .rspec-metadata: &rspec-metadata <<: *dedicated-runner <<: *pull-cache <<: *except-docs + <<: *except-qa stage: test script: - JOB_NAME=( $CI_JOB_NAME ) @@ -118,6 +123,7 @@ stages: <<: *dedicated-runner <<: *pull-cache <<: *except-docs + <<: *except-qa stage: test script: - JOB_NAME=( $CI_JOB_NAME ) @@ -169,6 +175,7 @@ package-qa: # Review docs base .review-docs: &review-docs + <<: *except-qa image: ruby:2.4-alpine before_script: - gem install gitlab --no-doc @@ -214,6 +221,7 @@ review-docs-cleanup: retrieve-tests-metadata: <<: *tests-metadata-state <<: *except-docs + <<: *except-qa stage: prepare cache: key: tests_metadata @@ -265,6 +273,7 @@ flaky-examples-check: except: - master - /(^docs[\/-].*|.*-docs$)/ + - /(^qa[\/-].*|.*-qa$)/ artifacts: expire_in: 30d paths: @@ -369,6 +378,7 @@ spinach-mysql 3 4: *spinach-metadata-mysql <<: *ruby-static-analysis <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: test script: @@ -387,6 +397,7 @@ static-analysis: # - Make sure cURL examples in API docs use the full switches docs lint: <<: *dedicated-runner + <<: *except-qa image: "registry.gitlab.com/gitlab-org/gitlab-build-images:nanoc-bootstrap-ruby-2.4-alpine" stage: test cache: {} @@ -409,6 +420,7 @@ downtime_check: - tags - /^[\d-]+-stable(-ee)?$/ - /(^docs[\/-].*|.*-docs$)/ + - /(^qa[\/-].*|.*-qa$)/ ee_compat_check: <<: *rake-exec @@ -430,6 +442,7 @@ ee_compat_check: .db-migrate-reset: &db-migrate-reset <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: test script: @@ -447,6 +460,7 @@ db:migrate:reset-mysql: <<: *dedicated-runner <<: *pull-cache <<: *except-docs + <<: *except-qa stage: test variables: SETUP_DB: "false" @@ -473,6 +487,7 @@ migration:path-mysql: .db-rollback: &db-rollback <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: test script: @@ -490,6 +505,7 @@ db:rollback-mysql: .db-seed_fu: &db-seed_fu <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: test variables: @@ -524,6 +540,7 @@ db:check-schema-pg: gitlab:assets:compile: <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: test dependencies: [] @@ -547,6 +564,7 @@ karma: <<: *use-pg <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: test variables: @@ -599,6 +617,7 @@ qa:internal: coverage: <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: post-test services: [] @@ -618,6 +637,7 @@ coverage: lint:javascript:report: <<: *dedicated-runner <<: *except-docs + <<: *except-qa <<: *pull-cache stage: post-test dependencies: @@ -677,6 +697,7 @@ cache gems: gitlab_git_test: <<: *pull-cache <<: *except-docs + <<: *except-qa variables: SETUP_DB: "false" script: 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/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/boards/components/board_sidebar.js b/app/assets/javascripts/boards/components/board_sidebar.js index faa76da964f..616de2347e1 100644 --- a/app/assets/javascripts/boards/components/board_sidebar.js +++ b/app/assets/javascripts/boards/components/board_sidebar.js @@ -1,9 +1,9 @@ /* eslint-disable comma-dangle, space-before-function-paren, no-new */ /* global MilestoneSelect */ -/* global Sidebar */ import Vue from 'vue'; import Flash from '../../flash'; +import Sidebar from '../../right_sidebar'; import eventHub from '../../sidebar/event_hub'; import assigneeTitle from '../../sidebar/components/assignees/assignee_title'; import assignees from '../../sidebar/components/assignees/assignees'; diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 5721375f4c6..62867c56214 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -11,7 +11,7 @@ import NotificationsForm from './notifications_form'; import notificationsDropdown from './notifications_dropdown'; import groupAvatar from './group_avatar'; import GroupLabelSubscription from './group_label_subscription'; -/* global LineHighlighter */ +import LineHighlighter from './line_highlighter'; import BuildArtifacts from './build_artifacts'; import CILintEditor from './ci_lint_editor'; import groupsSelect from './groups_select'; @@ -21,7 +21,7 @@ import NamespaceSelect from './namespace_select'; import NewCommitForm from './new_commit_form'; import Project from './project'; import projectAvatar from './project_avatar'; -/* global MergeRequest */ +import MergeRequest from './merge_request'; import Compare from './compare'; import initCompareAutocomplete from './compare_autocomplete'; import ProjectFindFile from './project_find_file'; @@ -29,12 +29,13 @@ import ProjectNew from './project_new'; import projectImport from './project_import'; import Labels from './labels'; import LabelManager from './label_manager'; -/* global Sidebar */ +import Sidebar from './right_sidebar'; import IssuableTemplateSelectors from './templates/issuable_template_selectors'; import Flash from './flash'; import CommitsList from './commits'; import Issue from './issue'; import BindInOut from './behaviors/bind_in_out'; +import SecretValues from './behaviors/secret_values'; import 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'; @@ -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': diff --git a/app/assets/javascripts/init_issuable_sidebar.js b/app/assets/javascripts/init_issuable_sidebar.js index ada693afc46..5d4c1851fe5 100644 --- a/app/assets/javascripts/init_issuable_sidebar.js +++ b/app/assets/javascripts/init_issuable_sidebar.js @@ -2,7 +2,7 @@ /* global MilestoneSelect */ import LabelsSelect from './labels_select'; import IssuableContext from './issuable_context'; -/* global Sidebar */ +import Sidebar from './right_sidebar'; import DueDateSelectors from './due_date_select'; @@ -15,5 +15,5 @@ export default () => { new LabelsSelect(); new IssuableContext(sidebarOptions.currentUser); new DueDateSelectors(); - window.sidebar = new Sidebar(); + Sidebar.initialize(); }; diff --git a/app/assets/javascripts/line_highlighter.js b/app/assets/javascripts/line_highlighter.js index a75d1a4b8d0..fbd381d8ff7 100644 --- a/app/assets/javascripts/line_highlighter.js +++ b/app/assets/javascripts/line_highlighter.js @@ -175,4 +175,4 @@ LineHighlighter.prototype.__setLocationHash__ = function(value) { }, document.title, value); }; -window.LineHighlighter = LineHighlighter; +export default LineHighlighter; diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 63f2a65700c..15a3a91c5f5 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -45,12 +45,10 @@ import './layout_nav'; import LazyLoader from './lazy_loader'; import './line_highlighter'; import initLogoAnimation from './logo'; -import './merge_request'; import './milestone_select'; import './preview_markdown'; import './projects_dropdown'; import './render_gfm'; -import './right_sidebar'; import initBreadcrumbs from './breadcrumb'; import './dispatcher'; diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index 6946c0b30f0..cb3cdea8111 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -1,5 +1,4 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, quotes, no-underscore-dangle, one-var, one-var-declaration-per-line, consistent-return, dot-notation, quote-props, comma-dangle, object-shorthand, max-len, prefer-arrow-callback */ -/* global MergeRequestTabs */ import 'vendor/jquery.waitforimages'; import TaskList from './task_list'; @@ -7,142 +6,138 @@ import MergeRequestTabs from './merge_request_tabs'; import IssuablesHelper from './helpers/issuables_helper'; import { addDelimiter } from './lib/utils/text_utility'; -(function() { - this.MergeRequest = (function() { - function MergeRequest(opts) { - // Initialize MergeRequest behavior - // - // Options: - // action - String, current controller action - // - this.opts = opts != null ? opts : {}; - this.submitNoteForm = this.submitNoteForm.bind(this); - this.$el = $('.merge-request'); - this.$('.show-all-commits').on('click', (function(_this) { - return function() { - return _this.showAllCommits(); - }; - })(this)); - - this.initTabs(); - this.initMRBtnListeners(); - this.initCommitMessageListeners(); - this.closeReopenReportToggle = IssuablesHelper.initCloseReopenReport(); - - if ($("a.btn-close").length) { - this.taskList = new TaskList({ - dataType: 'merge_request', - fieldName: 'description', - selector: '.detail-page-description', - onSuccess: (result) => { - document.querySelector('#task_status').innerText = result.task_status; - document.querySelector('#task_status_short').innerText = result.task_status_short; - } - }); - } - } - - // Local jQuery finder - MergeRequest.prototype.$ = function(selector) { - return this.$el.find(selector); +function MergeRequest(opts) { + // Initialize MergeRequest behavior + // + // Options: + // action - String, current controller action + // + this.opts = opts != null ? opts : {}; + this.submitNoteForm = this.submitNoteForm.bind(this); + this.$el = $('.merge-request'); + this.$('.show-all-commits').on('click', (function(_this) { + return function() { + return _this.showAllCommits(); }; - - MergeRequest.prototype.initTabs = function() { - if (window.mrTabs) { - window.mrTabs.unbindEvents(); + })(this)); + + this.initTabs(); + this.initMRBtnListeners(); + this.initCommitMessageListeners(); + this.closeReopenReportToggle = IssuablesHelper.initCloseReopenReport(); + + if ($("a.btn-close").length) { + this.taskList = new TaskList({ + dataType: 'merge_request', + fieldName: 'description', + selector: '.detail-page-description', + onSuccess: (result) => { + document.querySelector('#task_status').innerText = result.task_status; + document.querySelector('#task_status_short').innerText = result.task_status_short; } - window.mrTabs = new MergeRequestTabs(this.opts); - }; - - MergeRequest.prototype.showAllCommits = function() { - this.$('.first-commits').remove(); - return this.$('.all-commits').removeClass('hide'); - }; - - MergeRequest.prototype.initMRBtnListeners = function() { - var _this; - _this = this; - return $('a.btn-close, a.btn-reopen').on('click', function(e) { - var $this, shouldSubmit; - $this = $(this); - shouldSubmit = $this.hasClass('btn-comment'); - if (shouldSubmit && $this.data('submitted')) { - return; - } - - if (this.closeReopenReportToggle) this.closeReopenReportToggle.setDisable(); - - if (shouldSubmit) { - if ($this.hasClass('btn-comment-and-close') || $this.hasClass('btn-comment-and-reopen')) { - e.preventDefault(); - e.stopImmediatePropagation(); - - _this.submitNoteForm($this.closest('form'), $this); - } - } - }); - }; - - MergeRequest.prototype.submitNoteForm = function(form, $button) { - var noteText; - noteText = form.find("textarea.js-note-text").val(); - if (noteText.trim().length > 0) { - form.submit(); - $button.data('submitted', true); - return $button.trigger('click'); - } - }; - - MergeRequest.prototype.initCommitMessageListeners = function() { - $(document).on('click', 'a.js-with-description-link', function(e) { - var textarea = $('textarea.js-commit-message'); - e.preventDefault(); + }); + } +} + +// Local jQuery finder +MergeRequest.prototype.$ = function(selector) { + return this.$el.find(selector); +}; + +MergeRequest.prototype.initTabs = function() { + if (window.mrTabs) { + window.mrTabs.unbindEvents(); + } + window.mrTabs = new MergeRequestTabs(this.opts); +}; + +MergeRequest.prototype.showAllCommits = function() { + this.$('.first-commits').remove(); + return this.$('.all-commits').removeClass('hide'); +}; + +MergeRequest.prototype.initMRBtnListeners = function() { + var _this; + _this = this; + return $('a.btn-close, a.btn-reopen').on('click', function(e) { + var $this, shouldSubmit; + $this = $(this); + shouldSubmit = $this.hasClass('btn-comment'); + if (shouldSubmit && $this.data('submitted')) { + return; + } - textarea.val(textarea.data('messageWithDescription')); - $('.js-with-description-hint').hide(); - $('.js-without-description-hint').show(); - }); + if (this.closeReopenReportToggle) this.closeReopenReportToggle.setDisable(); - $(document).on('click', 'a.js-without-description-link', function(e) { - var textarea = $('textarea.js-commit-message'); + if (shouldSubmit) { + if ($this.hasClass('btn-comment-and-close') || $this.hasClass('btn-comment-and-reopen')) { e.preventDefault(); + e.stopImmediatePropagation(); - textarea.val(textarea.data('messageWithoutDescription')); - $('.js-with-description-hint').show(); - $('.js-without-description-hint').hide(); - }); - }; - - MergeRequest.prototype.updateStatusText = function(classToRemove, classToAdd, newStatusText) { - $('.detail-page-header .status-box') - .removeClass(classToRemove) - .addClass(classToAdd) - .find('span') - .text(newStatusText); - }; - - MergeRequest.prototype.decreaseCounter = function(by = 1) { - const $el = $('.nav-links .js-merge-counter'); - const count = Math.max((parseInt($el.text().replace(/[^\d]/, ''), 10) - by), 0); - - $el.text(addDelimiter(count)); - }; - - MergeRequest.prototype.hideCloseButton = function() { - const el = document.querySelector('.merge-request .js-issuable-actions'); - const closeDropdownItem = el.querySelector('li.close-item'); - if (closeDropdownItem) { - closeDropdownItem.classList.add('hidden'); - // Selects the next dropdown item - el.querySelector('li.report-item').click(); - } else { - // No dropdown just hide the Close button - el.querySelector('.btn-close').classList.add('hidden'); + _this.submitNoteForm($this.closest('form'), $this); } - // Dropdown for mobile screen - el.querySelector('li.js-close-item').classList.add('hidden'); - }; - - return MergeRequest; - })(); -}).call(window); + } + }); +}; + +MergeRequest.prototype.submitNoteForm = function(form, $button) { + var noteText; + noteText = form.find("textarea.js-note-text").val(); + if (noteText.trim().length > 0) { + form.submit(); + $button.data('submitted', true); + return $button.trigger('click'); + } +}; + +MergeRequest.prototype.initCommitMessageListeners = function() { + $(document).on('click', 'a.js-with-description-link', function(e) { + var textarea = $('textarea.js-commit-message'); + e.preventDefault(); + + textarea.val(textarea.data('messageWithDescription')); + $('.js-with-description-hint').hide(); + $('.js-without-description-hint').show(); + }); + + $(document).on('click', 'a.js-without-description-link', function(e) { + var textarea = $('textarea.js-commit-message'); + e.preventDefault(); + + textarea.val(textarea.data('messageWithoutDescription')); + $('.js-with-description-hint').show(); + $('.js-without-description-hint').hide(); + }); +}; + +MergeRequest.prototype.updateStatusText = function(classToRemove, classToAdd, newStatusText) { + $('.detail-page-header .status-box') + .removeClass(classToRemove) + .addClass(classToAdd) + .find('span') + .text(newStatusText); +}; + +MergeRequest.prototype.decreaseCounter = function(by = 1) { + const $el = $('.nav-links .js-merge-counter'); + const count = Math.max((parseInt($el.text().replace(/[^\d]/, ''), 10) - by), 0); + + $el.text(addDelimiter(count)); +}; + +MergeRequest.prototype.hideCloseButton = function() { + const el = document.querySelector('.merge-request .js-issuable-actions'); + const closeDropdownItem = el.querySelector('li.close-item'); + if (closeDropdownItem) { + closeDropdownItem.classList.add('hidden'); + // Selects the next dropdown item + el.querySelector('li.report-item').click(); + } else { + // No dropdown just hide the Close button + el.querySelector('.btn-close').classList.add('hidden'); + } + // Dropdown for mobile screen + el.querySelector('li.js-close-item').classList.add('hidden'); +}; + +export default MergeRequest; 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/repo/components/repo_preview.vue b/app/assets/javascripts/repo/components/repo_preview.vue index 425c55fafb5..3d1e0297bd5 100644 --- a/app/assets/javascripts/repo/components/repo_preview.vue +++ b/app/assets/javascripts/repo/components/repo_preview.vue @@ -1,6 +1,6 @@ <script> -/* global LineHighlighter */ import { mapGetters } from 'vuex'; +import LineHighlighter from '../../line_highlighter'; import syntaxHighlight from '../../syntax_highlight'; export default { diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index ec85b8b6529..b830fcf7e80 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -3,226 +3,228 @@ import _ from 'underscore'; import Cookies from 'js-cookie'; -(function() { - this.Sidebar = (function() { - function Sidebar(currentUser) { - this.toggleTodo = this.toggleTodo.bind(this); - this.sidebar = $('aside'); - - this.removeListeners(); - this.addEventListeners(); +function Sidebar(currentUser) { + this.toggleTodo = this.toggleTodo.bind(this); + this.sidebar = $('aside'); + + this.removeListeners(); + this.addEventListeners(); +} + +Sidebar.initialize = function(currentUser) { + if (!this.instance) { + this.instance = new Sidebar(currentUser); + } +}; + +Sidebar.prototype.removeListeners = function () { + this.sidebar.off('click', '.sidebar-collapsed-icon'); + this.sidebar.off('hidden.gl.dropdown'); + $('.dropdown').off('loading.gl.dropdown'); + $('.dropdown').off('loaded.gl.dropdown'); + $(document).off('click', '.js-sidebar-toggle'); +}; + +Sidebar.prototype.addEventListeners = function() { + const $document = $(document); + + this.sidebar.on('click', '.sidebar-collapsed-icon', this, this.sidebarCollapseClicked); + this.sidebar.on('hidden.gl.dropdown', this, this.onSidebarDropdownHidden); + $('.dropdown').on('loading.gl.dropdown', this.sidebarDropdownLoading); + $('.dropdown').on('loaded.gl.dropdown', this.sidebarDropdownLoaded); + + $document.on('click', '.js-sidebar-toggle', this.sidebarToggleClicked); + return $(document).off('click', '.js-issuable-todo').on('click', '.js-issuable-todo', this.toggleTodo); +}; + +Sidebar.prototype.sidebarToggleClicked = function (e, triggered) { + var $allGutterToggleIcons, $this, $thisIcon; + e.preventDefault(); + $this = $(this); + $thisIcon = $this.find('i'); + $allGutterToggleIcons = $('.js-sidebar-toggle i'); + if ($thisIcon.hasClass('fa-angle-double-right')) { + $allGutterToggleIcons.removeClass('fa-angle-double-right').addClass('fa-angle-double-left'); + $('aside.right-sidebar').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); + $('.layout-page').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); + } else { + $allGutterToggleIcons.removeClass('fa-angle-double-left').addClass('fa-angle-double-right'); + $('aside.right-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); + $('.layout-page').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); + + if (gl.lazyLoader) gl.lazyLoader.loadCheck(); + } + if (!triggered) { + Cookies.set("collapsed_gutter", $('.right-sidebar').hasClass('right-sidebar-collapsed')); + } +}; + +Sidebar.prototype.toggleTodo = function(e) { + var $btnText, $this, $todoLoading, ajaxType, url; + $this = $(e.currentTarget); + ajaxType = $this.attr('data-delete-path') ? 'DELETE' : 'POST'; + if ($this.attr('data-delete-path')) { + url = "" + ($this.attr('data-delete-path')); + } else { + url = "" + ($this.data('url')); + } + + $this.tooltip('hide'); + + return $.ajax({ + url: url, + type: ajaxType, + dataType: 'json', + data: { + issuable_id: $this.data('issuable-id'), + issuable_type: $this.data('issuable-type') + }, + beforeSend: (function(_this) { + return function() { + $('.js-issuable-todo').disable() + .addClass('is-loading'); + }; + })(this) + }).done((function(_this) { + return function(data) { + return _this.todoUpdateDone(data); + }; + })(this)); +}; + +Sidebar.prototype.todoUpdateDone = function(data) { + const deletePath = data.delete_path ? data.delete_path : null; + const attrPrefix = deletePath ? 'mark' : 'todo'; + const $todoBtns = $('.js-issuable-todo'); + + $(document).trigger('todo:toggle', data.count); + + $todoBtns.each((i, el) => { + const $el = $(el); + const $elText = $el.find('.js-issuable-todo-inner'); + + $el.removeClass('is-loading') + .enable() + .attr('aria-label', $el.data(`${attrPrefix}-text`)) + .attr('data-delete-path', deletePath) + .attr('title', $el.data(`${attrPrefix}-text`)); + + if ($el.hasClass('has-tooltip')) { + $el.tooltip('fixTitle'); } - Sidebar.prototype.removeListeners = function () { - this.sidebar.off('click', '.sidebar-collapsed-icon'); - this.sidebar.off('hidden.gl.dropdown'); - $('.dropdown').off('loading.gl.dropdown'); - $('.dropdown').off('loaded.gl.dropdown'); - $(document).off('click', '.js-sidebar-toggle'); - }; - - Sidebar.prototype.addEventListeners = function() { - const $document = $(document); - - this.sidebar.on('click', '.sidebar-collapsed-icon', this, this.sidebarCollapseClicked); - this.sidebar.on('hidden.gl.dropdown', this, this.onSidebarDropdownHidden); - $('.dropdown').on('loading.gl.dropdown', this.sidebarDropdownLoading); - $('.dropdown').on('loaded.gl.dropdown', this.sidebarDropdownLoaded); - - $document.on('click', '.js-sidebar-toggle', this.sidebarToggleClicked); - return $(document).off('click', '.js-issuable-todo').on('click', '.js-issuable-todo', this.toggleTodo); - }; - - Sidebar.prototype.sidebarToggleClicked = function (e, triggered) { - var $allGutterToggleIcons, $this, $thisIcon; - e.preventDefault(); - $this = $(this); - $thisIcon = $this.find('i'); - $allGutterToggleIcons = $('.js-sidebar-toggle i'); - if ($thisIcon.hasClass('fa-angle-double-right')) { - $allGutterToggleIcons.removeClass('fa-angle-double-right').addClass('fa-angle-double-left'); - $('aside.right-sidebar').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); - $('.layout-page').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); - } else { - $allGutterToggleIcons.removeClass('fa-angle-double-left').addClass('fa-angle-double-right'); - $('aside.right-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); - $('.layout-page').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); - - if (gl.lazyLoader) gl.lazyLoader.loadCheck(); - } - if (!triggered) { - Cookies.set("collapsed_gutter", $('.right-sidebar').hasClass('right-sidebar-collapsed')); - } - }; - - Sidebar.prototype.toggleTodo = function(e) { - var $btnText, $this, $todoLoading, ajaxType, url; - $this = $(e.currentTarget); - ajaxType = $this.attr('data-delete-path') ? 'DELETE' : 'POST'; - if ($this.attr('data-delete-path')) { - url = "" + ($this.attr('data-delete-path')); - } else { - url = "" + ($this.data('url')); - } - - $this.tooltip('hide'); - - return $.ajax({ - url: url, - type: ajaxType, - dataType: 'json', - data: { - issuable_id: $this.data('issuable-id'), - issuable_type: $this.data('issuable-type') - }, - beforeSend: (function(_this) { - return function() { - $('.js-issuable-todo').disable() - .addClass('is-loading'); - }; - })(this) - }).done((function(_this) { - return function(data) { - return _this.todoUpdateDone(data); - }; - })(this)); - }; - - Sidebar.prototype.todoUpdateDone = function(data) { - const deletePath = data.delete_path ? data.delete_path : null; - const attrPrefix = deletePath ? 'mark' : 'todo'; - const $todoBtns = $('.js-issuable-todo'); - - $(document).trigger('todo:toggle', data.count); - - $todoBtns.each((i, el) => { - const $el = $(el); - const $elText = $el.find('.js-issuable-todo-inner'); - - $el.removeClass('is-loading') - .enable() - .attr('aria-label', $el.data(`${attrPrefix}-text`)) - .attr('data-delete-path', deletePath) - .attr('title', $el.data(`${attrPrefix}-text`)); - - if ($el.hasClass('has-tooltip')) { - $el.tooltip('fixTitle'); - } - - if ($el.data(`${attrPrefix}-icon`)) { - $elText.html($el.data(`${attrPrefix}-icon`)); - } else { - $elText.text($el.data(`${attrPrefix}-text`)); - } - }); - }; - - Sidebar.prototype.sidebarDropdownLoading = function(e) { - var $loading, $sidebarCollapsedIcon, i, img; - $sidebarCollapsedIcon = $(this).closest('.block').find('.sidebar-collapsed-icon'); - img = $sidebarCollapsedIcon.find('img'); - i = $sidebarCollapsedIcon.find('i'); - $loading = $('<i class="fa fa-spinner fa-spin"></i>'); - if (img.length) { - img.before($loading); - return img.hide(); - } else if (i.length) { - i.before($loading); - return i.hide(); - } - }; - - Sidebar.prototype.sidebarDropdownLoaded = function(e) { - var $sidebarCollapsedIcon, i, img; - $sidebarCollapsedIcon = $(this).closest('.block').find('.sidebar-collapsed-icon'); - img = $sidebarCollapsedIcon.find('img'); - $sidebarCollapsedIcon.find('i.fa-spin').remove(); - i = $sidebarCollapsedIcon.find('i'); - if (img.length) { - return img.show(); - } else { - return i.show(); - } - }; - - Sidebar.prototype.sidebarCollapseClicked = function(e) { - var $block, sidebar; - if ($(e.currentTarget).hasClass('dont-change-state')) { - return; - } - sidebar = e.data; - e.preventDefault(); - $block = $(this).closest('.block'); - return sidebar.openDropdown($block); - }; - - Sidebar.prototype.openDropdown = function(blockOrName) { - var $block; - $block = _.isString(blockOrName) ? this.getBlock(blockOrName) : blockOrName; - if (!this.isOpen()) { - this.setCollapseAfterUpdate($block); - this.toggleSidebar('open'); - } - - // Wait for the sidebar to trigger('click') open - // so it doesn't cause our dropdown to close preemptively - setTimeout(() => { - $block.find('.js-sidebar-dropdown-toggle').trigger('click'); - }); - }; - - Sidebar.prototype.setCollapseAfterUpdate = function($block) { - $block.addClass('collapse-after-update'); - return $('.layout-page').addClass('with-overlay'); - }; - - Sidebar.prototype.onSidebarDropdownHidden = function(e) { - var $block, sidebar; - sidebar = e.data; - e.preventDefault(); - $block = $(e.target).closest('.block'); - return sidebar.sidebarDropdownHidden($block); - }; - - Sidebar.prototype.sidebarDropdownHidden = function($block) { - if ($block.hasClass('collapse-after-update')) { - $block.removeClass('collapse-after-update'); - $('.layout-page').removeClass('with-overlay'); - return this.toggleSidebar('hide'); - } - }; - - Sidebar.prototype.triggerOpenSidebar = function() { - return this.sidebar.find('.js-sidebar-toggle').trigger('click'); - }; - - Sidebar.prototype.toggleSidebar = function(action) { - if (action == null) { - action = 'toggle'; - } - if (action === 'toggle') { - this.triggerOpenSidebar(); - } - if (action === 'open') { - if (!this.isOpen()) { - this.triggerOpenSidebar(); - } - } - if (action === 'hide') { - if (this.isOpen()) { - return this.triggerOpenSidebar(); - } - } - }; + if ($el.data(`${attrPrefix}-icon`)) { + $elText.html($el.data(`${attrPrefix}-icon`)); + } else { + $elText.text($el.data(`${attrPrefix}-text`)); + } + }); +}; + +Sidebar.prototype.sidebarDropdownLoading = function(e) { + var $loading, $sidebarCollapsedIcon, i, img; + $sidebarCollapsedIcon = $(this).closest('.block').find('.sidebar-collapsed-icon'); + img = $sidebarCollapsedIcon.find('img'); + i = $sidebarCollapsedIcon.find('i'); + $loading = $('<i class="fa fa-spinner fa-spin"></i>'); + if (img.length) { + img.before($loading); + return img.hide(); + } else if (i.length) { + i.before($loading); + return i.hide(); + } +}; + +Sidebar.prototype.sidebarDropdownLoaded = function(e) { + var $sidebarCollapsedIcon, i, img; + $sidebarCollapsedIcon = $(this).closest('.block').find('.sidebar-collapsed-icon'); + img = $sidebarCollapsedIcon.find('img'); + $sidebarCollapsedIcon.find('i.fa-spin').remove(); + i = $sidebarCollapsedIcon.find('i'); + if (img.length) { + return img.show(); + } else { + return i.show(); + } +}; + +Sidebar.prototype.sidebarCollapseClicked = function(e) { + var $block, sidebar; + if ($(e.currentTarget).hasClass('dont-change-state')) { + return; + } + sidebar = e.data; + e.preventDefault(); + $block = $(this).closest('.block'); + return sidebar.openDropdown($block); +}; + +Sidebar.prototype.openDropdown = function(blockOrName) { + var $block; + $block = _.isString(blockOrName) ? this.getBlock(blockOrName) : blockOrName; + if (!this.isOpen()) { + this.setCollapseAfterUpdate($block); + this.toggleSidebar('open'); + } + + // Wait for the sidebar to trigger('click') open + // so it doesn't cause our dropdown to close preemptively + setTimeout(() => { + $block.find('.js-sidebar-dropdown-toggle').trigger('click'); + }); +}; + +Sidebar.prototype.setCollapseAfterUpdate = function($block) { + $block.addClass('collapse-after-update'); + return $('.layout-page').addClass('with-overlay'); +}; + +Sidebar.prototype.onSidebarDropdownHidden = function(e) { + var $block, sidebar; + sidebar = e.data; + e.preventDefault(); + $block = $(e.target).closest('.block'); + return sidebar.sidebarDropdownHidden($block); +}; + +Sidebar.prototype.sidebarDropdownHidden = function($block) { + if ($block.hasClass('collapse-after-update')) { + $block.removeClass('collapse-after-update'); + $('.layout-page').removeClass('with-overlay'); + return this.toggleSidebar('hide'); + } +}; + +Sidebar.prototype.triggerOpenSidebar = function() { + return this.sidebar.find('.js-sidebar-toggle').trigger('click'); +}; + +Sidebar.prototype.toggleSidebar = function(action) { + if (action == null) { + action = 'toggle'; + } + if (action === 'toggle') { + this.triggerOpenSidebar(); + } + if (action === 'open') { + if (!this.isOpen()) { + this.triggerOpenSidebar(); + } + } + if (action === 'hide') { + if (this.isOpen()) { + return this.triggerOpenSidebar(); + } + } +}; - Sidebar.prototype.isOpen = function() { - return this.sidebar.is('.right-sidebar-expanded'); - }; +Sidebar.prototype.isOpen = function() { + return this.sidebar.is('.right-sidebar-expanded'); +}; - Sidebar.prototype.getBlock = function(name) { - return this.sidebar.find(".block." + name); - }; +Sidebar.prototype.getBlock = function(name) { + return this.sidebar.find(".block." + name); +}; - return Sidebar; - })(); -}).call(window); +export default Sidebar; diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 305f97b010e..292e3d6a657 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -1,8 +1,8 @@ /* global Mousetrap */ -/* global sidebar */ import _ from 'underscore'; import 'mousetrap'; +import Sidebar from './right_sidebar'; import ShortcutsNavigation from './shortcuts_navigation'; import { CopyAsGFM } from './behaviors/copy_as_gfm'; @@ -69,7 +69,7 @@ export default class ShortcutsIssuable extends ShortcutsNavigation { } static openSidebarDropdown(name) { - sidebar.openDropdown(name); + Sidebar.instance.openDropdown(name); return false; } } 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/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/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/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index eebbf7c4218..28f154581a9 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -228,6 +228,10 @@ module Ci statuses.select(:stage).distinct.count end + def total_size + statuses.count(:id) + end + def stages_names statuses.order(:stage_idx).distinct .pluck(:stage, :stage_idx).map(&:first) 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/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/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/notify/pipeline_success_email.html.haml b/app/views/notify/pipeline_success_email.html.haml index 574a8f2fa50..bae37292d62 100644 --- a/app/views/notify/pipeline_success_email.html.haml +++ b/app/views/notify/pipeline_success_email.html.haml @@ -109,7 +109,7 @@ API %tr %td{ colspan: 2, style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;color:#333333;font-size:15px;font-weight:300;line-height:1.4;padding:15px 5px;text-align:center;" } - - job_count = @pipeline.statuses.latest.size + - job_count = @pipeline.total_size - stage_count = @pipeline.stages_count successfully completed #{job_count} #{'job'.pluralize(job_count)} diff --git a/app/views/notify/pipeline_success_email.text.erb b/app/views/notify/pipeline_success_email.text.erb index ddced2279e1..39622cf7f02 100644 --- a/app/views/notify/pipeline_success_email.text.erb +++ b/app/views/notify/pipeline_success_email.text.erb @@ -22,11 +22,11 @@ Committed by: <%= commit.committer_name %> <% end -%> <% end -%> -<% build_count = @pipeline.statuses.latest.size -%> +<% job_count = @pipeline.total_size -%> <% stage_count = @pipeline.stages_count -%> <% if @pipeline.user -%> Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by <%= @pipeline.user.name %> ( <%= user_url(@pipeline.user) %> ) <% else -%> Pipeline #<%= @pipeline.id %> ( <%= pipeline_url(@pipeline) %> ) triggered by API <% end -%> -successfully completed <%= build_count %> <%= 'build'.pluralize(build_count) %> in <%= stage_count %> <%= 'stage'.pluralize(stage_count) %>. +successfully completed <%= job_count %> <%= 'job'.pluralize(job_count) %> in <%= stage_count %> <%= 'stage'.pluralize(stage_count) %>. 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/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index f5149306734..01ea9356af5 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -13,7 +13,7 @@ .well-segment.pipeline-info .icon-container = icon('clock-o') - = pluralize @pipeline.statuses.count(:id), "job" + = pluralize @pipeline.total_size, "job" - if @pipeline.ref from = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" diff --git a/app/views/projects/pipelines/_with_tabs.html.haml b/app/views/projects/pipelines/_with_tabs.html.haml index ad61f033a1c..398a1c46746 100644 --- a/app/views/projects/pipelines/_with_tabs.html.haml +++ b/app/views/projects/pipelines/_with_tabs.html.haml @@ -8,7 +8,7 @@ %li.js-builds-tab-link = link_to builds_project_pipeline_path(@project, @pipeline), data: {target: 'div#js-tab-builds', action: 'builds', toggle: 'tab' }, class: 'builds-tab' do Jobs - %span.badge.js-builds-counter= pipeline.statuses.count + %span.badge.js-builds-counter= pipeline.total_size - if failed_builds.present? %li.js-failures-tab-link = link_to failures_project_pipeline_path(@project, @pipeline), data: {target: 'div#js-tab-failures', action: 'failures', toggle: 'tab' }, class: 'failures-tab' do 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/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/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/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/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/fix_build_count_in_pipeline_success_maild.yml b/changelogs/unreleased/fix_build_count_in_pipeline_success_maild.yml new file mode 100644 index 00000000000..c39bba62271 --- /dev/null +++ b/changelogs/unreleased/fix_build_count_in_pipeline_success_maild.yml @@ -0,0 +1,5 @@ +--- +title: fix build count in pipeline success mail +merge_request: 15827 +author: Christiaan Van den Poel +type: fixed 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 25c5a8f5a99..1110199b888 100644 --- a/config/application.rb +++ b/config/application.rb @@ -163,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/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/articles/laravel_with_gitlab_and_envoy/index.md b/doc/articles/laravel_with_gitlab_and_envoy/index.md index e0d8fb8d081..b20bd8c247a 100644 --- a/doc/articles/laravel_with_gitlab_and_envoy/index.md +++ b/doc/articles/laravel_with_gitlab_and_envoy/index.md @@ -502,8 +502,8 @@ stages: unit_test: stage: test script: - - composer install - cp .env.example .env + - composer install - php artisan key:generate - php artisan migrate - vendor/bin/phpunit diff --git a/doc/development/README.md b/doc/development/README.md index c9ffa912d51..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) 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/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/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 27c712a84d4..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"] @@ -130,14 +131,15 @@ module ExtractsPath 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) + @lfs_blob_ids = Gitlab::Git::Blob.batch_lfs_pointers(@project.repository, blob_ids).map(&:id) # rubocop:disable Gitlab/ModuleWithInstanceVariables end private @@ -150,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/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/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/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/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/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/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/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/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/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/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 888e290292b..3987cea0b4f 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -152,7 +152,7 @@ describe 'Pipeline', :js do end it 'shows counter in Jobs tab' do - expect(page.find('.js-builds-counter').text).to eq(pipeline.statuses.count.to_s) + expect(page.find('.js-builds-counter').text).to eq(pipeline.total_size.to_s) end it 'shows Pipeline tab as active' do @@ -248,7 +248,7 @@ describe 'Pipeline', :js do end it 'shows counter in Jobs tab' do - expect(page.find('.js-builds-counter').text).to eq(pipeline.statuses.count.to_s) + expect(page.find('.js-builds-counter').text).to eq(pipeline.total_size.to_s) end it 'shows Jobs tab as active' do 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/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/collapsed_sidebar_todo_spec.js b/spec/javascripts/collapsed_sidebar_todo_spec.js index 974815fe939..5026eaafaca 100644 --- a/spec/javascripts/collapsed_sidebar_todo_spec.js +++ b/spec/javascripts/collapsed_sidebar_todo_spec.js @@ -1,7 +1,6 @@ -/* global Sidebar */ /* eslint-disable no-new */ import _ from 'underscore'; -import '~/right_sidebar'; +import Sidebar from '~/right_sidebar'; describe('Issuable right sidebar collapsed todo toggle', () => { const fixtureName = 'issues/open-issue.html.raw'; diff --git a/spec/javascripts/line_highlighter_spec.js b/spec/javascripts/line_highlighter_spec.js index 645664a5219..89f4b85541d 100644 --- a/spec/javascripts/line_highlighter_spec.js +++ b/spec/javascripts/line_highlighter_spec.js @@ -1,7 +1,6 @@ /* eslint-disable space-before-function-paren, no-var, no-param-reassign, quotes, prefer-template, no-else-return, new-cap, dot-notation, no-return-assign, comma-dangle, no-new, one-var, one-var-declaration-per-line, jasmine/no-spec-dupes, no-underscore-dangle, max-len */ -/* global LineHighlighter */ -import '~/line_highlighter'; +import LineHighlighter from '~/line_highlighter'; (function() { describe('LineHighlighter', function() { diff --git a/spec/javascripts/merge_request_spec.js b/spec/javascripts/merge_request_spec.js index 70ae63ba036..2f02c11482f 100644 --- a/spec/javascripts/merge_request_spec.js +++ b/spec/javascripts/merge_request_spec.js @@ -1,7 +1,6 @@ /* eslint-disable space-before-function-paren, no-return-assign */ -/* global MergeRequest */ -import '~/merge_request'; +import MergeRequest from '~/merge_request'; import CloseReopenReportToggle from '~/close_reopen_report_toggle'; import IssuablesHelper from '~/helpers/issuables_helper'; diff --git a/spec/javascripts/right_sidebar_spec.js b/spec/javascripts/right_sidebar_spec.js index 72790eb215a..3267e29585b 100644 --- a/spec/javascripts/right_sidebar_spec.js +++ b/spec/javascripts/right_sidebar_spec.js @@ -1,8 +1,7 @@ /* eslint-disable space-before-function-paren, no-var, one-var, one-var-declaration-per-line, new-parens, no-return-assign, new-cap, vars-on-top, max-len */ -/* global Sidebar */ import '~/commons/bootstrap'; -import '~/right_sidebar'; +import Sidebar from '~/right_sidebar'; (function() { var $aside, $icon, $labelsIcon, $page, $toggle, assertSidebarState; 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/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index bb89e093890..a1f63a2534b 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 @@ -1530,4 +1530,16 @@ describe Ci::Pipeline, :mailer do expect(query_count).to eq(1) end end + + describe '#total_size' do + let!(:build_job1) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } + let!(:build_job2) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } + let!(:test_job_failed_and_retried) { create(:ci_build, :failed, :retried, pipeline: pipeline, stage_idx: 1) } + let!(:second_test_job) { create(:ci_build, pipeline: pipeline, stage_idx: 1) } + let!(:deploy_job) { create(:ci_build, pipeline: pipeline, stage_idx: 2) } + + it 'returns all jobs (including failed and retried)' do + expect(pipeline.total_size).to eq(5) + end + end end 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/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/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/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" |