Commit 0a05d874 authored by Phil Hughes's avatar Phil Hughes

Merge branch 'master' into 'ph-es-notes-module'

# Conflicts:
#   app/assets/javascripts/main.js
parents eaf2f48d b4ea25ca
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:
......
......@@ -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
......
......@@ -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'
......
......@@ -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)
......
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;
}
}
/* 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';
......
......@@ -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':
......
......@@ -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();
};
......@@ -175,4 +175,4 @@ LineHighlighter.prototype.__setLocationHash__ = function(value) {
}, document.title, value);
};
window.LineHighlighter = LineHighlighter;
export default LineHighlighter;
......@@ -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';
......
/* 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;
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);
}
}
<script>
/* global LineHighlighter */
import { mapGetters } from 'vuex';
import LineHighlighter from '../../line_highlighter';
import syntaxHighlight from '../../syntax_highlight';
export default {
......
This diff is collapsed.
/* 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;
}
}
<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>
......@@ -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;
......
......@@ -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;
}
}
}
......
......@@ -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)
......
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
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
......@@ -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
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
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
......@@ -14,6 +14,6 @@ module OauthApplications
end
def load_scopes
@scopes = Doorkeeper.configuration.scopes
@scopes ||= Doorkeeper.configuration.scopes
end
end
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
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
......
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
......
......@@ -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)
......
......@@ -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
......
......@@ -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)
......
......@@ -12,7 +12,7 @@ module ToggleSubscriptionAction
private
def subscribable_project
@project || raise(NotImplementedError)
@project ||= raise(NotImplementedError)
end
def subscribable_resource
......
......@@ -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
#
......
......@@ -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)
......
......@@ -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
......
......@@ -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
......
......@@ -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?)
......
......@@ -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)
......
......@@ -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
......@@ -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)
......
......@@ -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
......@@ -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. "\
......
......@@ -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
......
......@@ -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
......@@ -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
......
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)
......
......@@ -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
......@@ -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
......
......@@ -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)
......@@ -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
......
......@@ -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)}
......
......@@ -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) %>.
......@@ -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')
......@@ -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
......
......@@ -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"
......
......@@ -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
......
......@@ -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
......
......@@ -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)
......
---
title: Hide runner token in CI/CD settings page
merge_request:
author:
type: added
---
title: Update feature toggle design to use icons and make it i18n friendly
merge_request: 15904
author:
type: changed
---
title: Fix broken illustration images for monitoring page empty states
merge_request: 15889
author:
type: fixed
---
title: Fix related branches/Merge requests failing to load when the hostname setting
is changed
merge_request:
author:
type: fixed
---
title: Fix the fork project functionality for projects with hashed storage
merge_request: 15671
author:
type: fixed
---
title: Fix updateEndpoint undefined error for issue_show app root
merge_request: 15698
author:
type: fixed
---
title: Keep track of all circuitbreaker keys in a set
merge_request: 15613
author:
type: performance
---
title: Correctly link to a forked project from the new fork page.
merge_request: 15653
author:
type: fixed
---
title: Create a fork network for forks with a deleted source
merge_request: 15595
author:
type: fixed
---
title: fix build count in pipeline success mail
merge_request: 15827
author: Christiaan Van den Poel
type: fixed
---
title: Only load branch names for protected branch checks
merge_request:
author:
type: performance
---
title: Removed incorrect guidance stating blocked users will be removed from groups
and project as members
merge_request: 15947
author: CesarApodaca
type: fixed
---
title: Fix gitlab:import:repos Rake task moving repositories into the wrong location
merge_request:
author:
type: fixed
---
title: "Gracefully handle case when repository's root ref does not exist"
merge_request:
author:
type: fixed
---
title: Optimize API /groups/:id/projects by preloading associations
merge_request:
author:
type: performance
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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)
......
......@@ -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.
......
......@@ -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)
......
## 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.
......@@ -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
......
......@@ -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.
---
......
......@@ -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.
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -90,6 +90,12 @@ module Gitlab
def self.aspects
@aspects ||= []
end
private
def entries
@entries
end
end
end
end
......
......@@ -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
......
......@@ -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
......
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment