diff options
175 files changed, 707 insertions, 436 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f55bdc19eec..e10958b3bee 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -514,8 +514,11 @@ codeclimate: services: - docker:dind script: + - cp .rubocop.yml .rubocop.yml.bak + - grep -v "rubocop-gitlab-security" .rubocop.yml.bak > .rubocop.yml - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate analyze -f json > raw_codeclimate.json - cat raw_codeclimate.json | docker run -i stedolan/jq -c 'map({check_name,fingerprint,location})' > codeclimate.json + - mv .rubocop.yml.bak .rubocop.yml artifacts: paths: [codeclimate.json] diff --git a/.rubocop.yml b/.rubocop.yml index a5ccec0437b..84e4a3c2e49 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,6 @@ require: - rubocop-rspec + - rubocop-gitlab-security - ./rubocop/rubocop inherit_from: .rubocop_todo.yml @@ -206,6 +207,13 @@ Layout/SpaceAroundKeyword: Layout/SpaceAroundOperators: Enabled: true +# Checks that block braces have or don't have a space before the opening +# brace depending on configuration. +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: space, no_space +Layout/SpaceBeforeBlockBraces: + Enabled: true + # No spaces before commas. Layout/SpaceBeforeComma: Enabled: true @@ -1156,3 +1164,35 @@ RSpec/SubjectStub: # Prefer using verifying doubles over normal doubles. RSpec/VerifiedDoubles: Enabled: false + +# GitlabSecurity ############################################################## + +GitlabSecurity/DeepMunge: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/PublicSend: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/RedirectToParamsUpdate: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/SqlInjection: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' + +GitlabSecurity/SystemCommandInjection: + Enabled: true + Exclude: + - 'spec/**/*' + - 'lib/**/*.rake' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9caef3bde08..78ab7f7204f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -26,13 +26,6 @@ Layout/IndentArray: Layout/IndentHash: Enabled: false -# Offense count: 174 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, SupportedStyles. -# SupportedStyles: space, no_space -Layout/SpaceBeforeBlockBraces: - Enabled: false - # Offense count: 8 # Cop supports --auto-correct. # Configuration parameters: AllowForAlignment. @@ -341,6 +341,7 @@ group :development, :test do gem 'rubocop', '~> 0.49.1', require: false gem 'rubocop-rspec', '~> 1.15.1', require: false + gem 'rubocop-gitlab-security', '~> 0.0.6', require: false gem 'scss_lint', '~> 0.54.0', require: false gem 'haml_lint', '~> 0.26.0', require: false gem 'simplecov', '~> 0.14.0', require: false @@ -354,7 +355,7 @@ group :development, :test do gem 'activerecord_sane_schema_dumper', '0.2' - gem 'stackprof', '~> 0.2.10' + gem 'stackprof', '~> 0.2.10', require: false end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 948ba02a72c..b865ff6ee32 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -548,7 +548,7 @@ GEM rubypants (~> 0.2) orm_adapter (0.5.0) os (0.9.6) - parallel (1.11.2) + parallel (1.12.0) paranoia (2.3.1) activerecord (>= 4.0, < 5.2) parser (2.4.0.0) @@ -741,8 +741,9 @@ GEM rainbow (>= 1.99.1, < 3.0) ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) + rubocop-gitlab-security (0.0.6) + rubocop (>= 0.47.1) rubocop-rspec (1.15.1) - rubocop (>= 0.42.0) ruby-fogbugz (0.2.1) crack (~> 0.4) ruby-prof (0.16.2) @@ -1088,6 +1089,7 @@ DEPENDENCIES rspec-set (~> 0.1.3) rspec_profiling (~> 0.0.5) rubocop (~> 0.49.1) + rubocop-gitlab-security (~> 0.0.6) rubocop-rspec (~> 1.15.1) ruby-fogbugz (~> 0.2.1) ruby-prof (~> 0.16.2) diff --git a/PROCESS.md b/PROCESS.md index 2b3d142bf77..e5b17784d20 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -119,6 +119,12 @@ only be left until after the freeze if: are aware of it. * It is in the correct milestone, with the ~Deliverable label. +If a merge request is not ready, but the developers and Product Manager +responsible for the feature think it is essential that it is in the release, +they can [ask for an exception](#asking-for-an-exception) in advance. This is +preferable to merging something that we are not confident in, but should still +be a rare case: most features can be allowed to slip a release. + All Community Edition merge requests from GitLab team members merged on the freeze date (the 7th) should have a corresponding Enterprise Edition merge request, even if there are no conflicts. This is to reduce the size of the @@ -128,11 +134,26 @@ information, see ### After the 7th -Once the stable branch is frozen, only fixes for [regressions](#regressions) -and security issues will be cherry-picked into the stable branch. -Any merge requests cherry-picked into the stable branch for a previous release will also be picked into the latest stable branch. -These fixes will be shipped in the next RC for that release if it is before the 22nd. -If the fixes are are completed on or after the 22nd, they will be shipped in a patch for that release. +Once the stable branch is frozen, the only MRs that can be cherry-picked into +the stable branch are: + +* Fixes for [regressions](#regressions) +* Fixes for security issues +* New or updated translations (as long as they do not touch application code) + +Any merge requests cherry-picked into the stable branch for a previous release +will also be picked into the latest stable branch. These fixes will be shipped +in the next RC for that release if it is before the 22nd. If the fixes are are +completed on or after the 22nd, they will be shipped in a patch for that +release. + +During the feature freeze all merge requests that are meant to go into the upcoming +release should have the correct milestone assigned _and_ have the label +~"Pick into Stable" set, so that release managers can find and pick them. +Merge requests without a milestone and this label will +not be merged into any stable branches. + +### Asking for an exception If you think a merge request should go into an RC or patch even though it does not meet these requirements, you can ask for an exception to be made. Exceptions require sign-off from 3 people besides the developer: @@ -152,11 +173,7 @@ When in doubt, we err on the side of _not_ cherry-picking. For example, it is likely that an exception will be made for a trivial 1-5 line performance improvement (e.g. adding a database index or adding `includes` to a query), but not for a new feature, no matter how relatively small or thoroughly tested. -During the feature freeze all merge requests that are meant to go into the upcoming -release should have the correct milestone assigned _and_ have the label -~"Pick into Stable" set, so that release managers can find and pick them. -Merge requests without a milestone and this label will -not be merged into any stable branches. +All MRs which have had exceptions granted must be merged by the 15th. ### Regressions diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index de3c30772d2..07f14b3ddb7 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -76,6 +76,7 @@ import initLegacyFilters from './init_legacy_filters'; import initIssuableSidebar from './init_issuable_sidebar'; import GpgBadges from './gpg_badges'; import UserFeatureHelper from './helpers/user_feature_helper'; +import initChangesDropdown from './init_changes_dropdown'; (function() { var Dispatcher; @@ -227,6 +228,7 @@ import UserFeatureHelper from './helpers/user_feature_helper'; break; case 'projects:compare:show': new gl.Diff(); + initChangesDropdown(); break; case 'projects:branches:new': case 'projects:branches:create': @@ -319,6 +321,7 @@ import UserFeatureHelper from './helpers/user_feature_helper'; container: '.js-commit-pipeline-graph', }).bindEvents(); initNotes(); + initChangesDropdown(); $('.commit-info.branches').load(document.querySelector('.js-commit-box').dataset.commitPath); break; case 'projects:commit:pipelines': diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index 2856c8e2862..ee71728184f 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -1,7 +1,7 @@ /* eslint-disable wrap-iife, func-names, space-before-function-paren, comma-dangle, prefer-template, consistent-return, class-methods-use-this, arrow-body-style, no-unused-vars, no-underscore-dangle, no-new, max-len, no-sequences, no-unused-expressions, no-param-reassign */ /* global dateFormat */ -/* global Pikaday */ +import Pikaday from 'pikaday'; import DateFix from './lib/utils/datefix'; class DueDateSelect { diff --git a/app/assets/javascripts/fly_out_nav.js b/app/assets/javascripts/fly_out_nav.js index 301e82f4610..ad957f132b8 100644 --- a/app/assets/javascripts/fly_out_nav.js +++ b/app/assets/javascripts/fly_out_nav.js @@ -1,7 +1,17 @@ /* global bp */ +import Cookies from 'js-cookie'; import './breakpoints'; -export const canShowSubItems = () => bp.getBreakpointSize() === 'md' || bp.getBreakpointSize() === 'lg'; +export const canShowActiveSubItems = (el) => { + const isHiddenByMedia = bp.getBreakpointSize() === 'sm' || bp.getBreakpointSize() === 'md'; + + if (el.classList.contains('active') && !isHiddenByMedia) { + return Cookies.get('sidebar_collapsed') === 'true'; + } + + return true; +}; +export const canShowSubItems = () => bp.getBreakpointSize() === 'sm' || bp.getBreakpointSize() === 'md' || bp.getBreakpointSize() === 'lg'; export const calculateTop = (boundingRect, outerHeight) => { const windowHeight = window.innerHeight; @@ -14,9 +24,10 @@ export const calculateTop = (boundingRect, outerHeight) => { export const showSubLevelItems = (el) => { const subItems = el.querySelector('.sidebar-sub-level-items'); - if (!subItems || !canShowSubItems()) return; + if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return; subItems.style.display = 'block'; + el.classList.add('is-showing-fly-out'); el.classList.add('is-over'); const boundingRect = el.getBoundingClientRect(); @@ -34,15 +45,17 @@ export const showSubLevelItems = (el) => { export const hideSubLevelItems = (el) => { const subItems = el.querySelector('.sidebar-sub-level-items'); - if (!subItems || !canShowSubItems()) return; + if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return; + el.classList.remove('is-showing-fly-out'); el.classList.remove('is-over'); - subItems.style.display = 'none'; + subItems.style.display = ''; + subItems.style.transform = ''; subItems.classList.remove('is-above'); }; export default () => { - const items = [...document.querySelectorAll('.sidebar-top-level-items > li:not(.active)')] + const items = [...document.querySelectorAll('.sidebar-top-level-items > li')] .filter(el => el.querySelector('.sidebar-sub-level-items')); items.forEach((el) => { diff --git a/app/assets/javascripts/init_changes_dropdown.js b/app/assets/javascripts/init_changes_dropdown.js new file mode 100644 index 00000000000..f785ed29e6c --- /dev/null +++ b/app/assets/javascripts/init_changes_dropdown.js @@ -0,0 +1,10 @@ +import stickyMonitor from './lib/utils/sticky'; + +export default () => { + stickyMonitor(document.querySelector('.js-diff-files-changed')); + + $('.js-diff-stats-dropdown').glDropdown({ + filterable: true, + remoteFilter: false, + }); +}; diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js index 9ac1325fc95..3f848e0859b 100644 --- a/app/assets/javascripts/issuable_form.js +++ b/app/assets/javascripts/issuable_form.js @@ -2,8 +2,8 @@ /* global GitLab */ /* global Autosave */ /* global dateFormat */ -/* global Pikaday */ +import Pikaday from 'pikaday'; import UsersSelect from './users_select'; import GfmAutoComplete from './gfm_auto_complete'; import ZenMode from './zen_mode'; diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 42092a34c2f..e0c61a474c6 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -7,7 +7,6 @@ import jQuery from 'jquery'; import _ from 'underscore'; import Cookies from 'js-cookie'; -import Pikaday from 'pikaday'; import Dropzone from 'dropzone'; import Sortable from 'vendor/Sortable'; @@ -20,7 +19,6 @@ import 'vendor/fuzzaldrin-plus'; window.jQuery = jQuery; window.$ = jQuery; window._ = _; -window.Pikaday = Pikaday; window.Dropzone = Dropzone; window.Sortable = Sortable; diff --git a/app/assets/javascripts/member_expiration_date.js b/app/assets/javascripts/member_expiration_date.js index e034729bd39..cc9016e74da 100644 --- a/app/assets/javascripts/member_expiration_date.js +++ b/app/assets/javascripts/member_expiration_date.js @@ -1,5 +1,7 @@ -/* global Pikaday */ /* global dateFormat */ + +import Pikaday from 'pikaday'; + (() => { // Add datepickers to all `js-access-expiration-date` elements. If those elements are // children of an element with the `clearable-input` class, and have a sibling diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 4ffd71d9de5..0294da3f20d 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -7,7 +7,7 @@ import Cookies from 'js-cookie'; import './breakpoints'; import './flash'; import BlobForkSuggestion from './blob/blob_fork_suggestion'; -import stickyMonitor from './lib/utils/sticky'; +import initChangesDropdown from './init_changes_dropdown'; /* eslint-disable max-len */ // MergeRequestTabs @@ -267,9 +267,7 @@ import stickyMonitor from './lib/utils/sticky'; const $container = $('#diffs'); $container.html(data.html); - this.initChangesDropdown(); - - stickyMonitor(document.querySelector('.js-diff-files-changed')); + initChangesDropdown(); if (typeof gl.diffNotesCompileComponents !== 'undefined') { gl.diffNotesCompileComponents(); @@ -319,13 +317,6 @@ import stickyMonitor from './lib/utils/sticky'; }); } - initChangesDropdown() { - $('.js-diff-stats-dropdown').glDropdown({ - filterable: true, - remoteFilter: false, - }); - } - // Show or hide the loading spinner // // status - Boolean, true to show, false to hide diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index 609bc9a7dfc..4367b8c1a15 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -104,11 +104,14 @@ $new-sidebar-collapsed-width: 50px; &.sidebar-icons-only { width: $new-sidebar-collapsed-width; - .nav-item-name, .badge, .project-title { display: none; } + + .nav-item-name { + opacity: 0; + } } &.nav-sidebar-expanded { @@ -182,7 +185,7 @@ $new-sidebar-collapsed-width: 50px; > li { a { - padding: 8px 16px 8px 50px; + padding: 8px 16px 8px 40px; &:hover, &:focus { @@ -215,12 +218,15 @@ $new-sidebar-collapsed-width: 50px; &:hover { color: $gl-text-color; + + svg { + fill: $gl-text-color; + } } } - &:not(.active) { + &.is-showing-fly-out { > a { - margin-left: 1px; margin-right: 2px; } @@ -271,6 +277,14 @@ $new-sidebar-collapsed-width: 50px; } } + > .active { + box-shadow: none; + + > a { + background-color: transparent; + } + } + a { padding: 8px 16px; color: $gl-text-color; @@ -294,6 +308,7 @@ $new-sidebar-collapsed-width: 50px; > a { margin-left: 4px; + padding-left: 12px; } .badge { @@ -354,7 +369,7 @@ $new-sidebar-collapsed-width: 50px; .sidebar-icons-only { .context-header { - height: 60px; + height: 61px; a { padding: 10px 4px; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index da77346d8b2..215bedc04fd 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -574,10 +574,14 @@ @media (min-width: $screen-sm-min) { position: -webkit-sticky; position: sticky; - top: 84px; + top: 34px; background-color: $white-light; z-index: 190; + &.diff-files-changed-merge-request { + top: 84px; + } + + .files, + .alert { margin-top: 1px; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index e165a583ecb..e2e603639ce 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -35,7 +35,7 @@ .commit-box, .info-well, .commit-ci-menu, - .files-changed, + .files-changed-inner, .limited-header-width, .limited-width-notes { @extend .fixed-width-container; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5b448008a1b..1d92ea11bda 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -117,7 +117,7 @@ class ApplicationController < ActionController::Base Raven.capture_exception(exception) if sentry_enabled? application_trace = ActionDispatch::ExceptionWrapper.new(env, exception).application_trace - application_trace.map!{ |t| " #{t}\n" } + application_trace.map! { |t| " #{t}\n" } logger.error "\n#{exception.class.name} (#{exception.message}):\n#{application_trace.join}" end diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index 53a5981e564..baa6645e5ce 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -68,15 +68,15 @@ class Import::GithubController < Import::BaseController end def new_import_url - public_send("new_import_#{provider}_url") + public_send("new_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend end def status_import_url - public_send("status_import_#{provider}_url") + public_send("status_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend end def callback_import_url - public_send("callback_import_#{provider}_url") + public_send("callback_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend end def provider_unauthorized diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb index 73837ffbe67..407154e59a0 100644 --- a/app/controllers/import/gitlab_controller.rb +++ b/app/controllers/import/gitlab_controller.rb @@ -15,7 +15,7 @@ class Import::GitlabController < Import::BaseController @already_added_projects = current_user.created_projects.where(import_type: "gitlab") already_added_projects_names = @already_added_projects.pluck(:import_source) - @repos = @repos.to_a.reject{ |repo| already_added_projects_names.include? repo["path_with_namespace"] } + @repos = @repos.to_a.reject { |repo| already_added_projects_names.include? repo["path_with_namespace"] } end def jobs diff --git a/app/helpers/graph_helper.rb b/app/helpers/graph_helper.rb index 2e9b72e9613..c53ea4519da 100644 --- a/app/helpers/graph_helper.rb +++ b/app/helpers/graph_helper.rb @@ -3,7 +3,7 @@ module GraphHelper refs = "" # Commit::ref_names already strips the refs/XXX from important refs (e.g. refs/heads/XXX) # so anything leftover is internally used by GitLab - commit_refs = commit.ref_names(repo).reject{ |name| name.starts_with?('refs/') } + commit_refs = commit.ref_names(repo).reject { |name| name.starts_with?('refs/') } refs << commit_refs.join(' ') # append note count diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index dc6fd31e271..4a3a03d0bfb 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -151,7 +151,7 @@ module IssuablesHelper end def issuable_labels_tooltip(labels, limit: 5) - first, last = labels.partition.with_index{ |_, i| i < limit } + first, last = labels.partition.with_index { |_, i| i < limit } label_names = first.collect(&:name) label_names << "and #{last.size} more" unless last.empty? @@ -234,7 +234,7 @@ module IssuablesHelper end def issuables_count_for_state(issuable_type, state, finder: nil) - finder ||= public_send("#{issuable_type}_finder") + finder ||= public_send("#{issuable_type}_finder") # rubocop:disable GitlabSecurity/PublicSend cache_key = finder.state_counter_cache_key @counts ||= {} @@ -329,7 +329,7 @@ module IssuablesHelper end def selected_template(issuable) - params[:issuable_template] if issuable_templates(issuable).any?{ |template| template[:name] == params[:issuable_template] } + params[:issuable_template] if issuable_templates(issuable).any? { |template| template[:name] == params[:issuable_template] } end def issuable_todo_button_data(issuable, todo, is_collapsed) diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 4b99de1b6a5..e60513b35c7 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -43,11 +43,11 @@ module LabelsHelper def label_filter_path(subject, label, type: :issue) case subject when Group - send("#{type.to_s.pluralize}_group_path", + send("#{type.to_s.pluralize}_group_path", # rubocop:disable GitlabSecurity/PublicSend subject, label_name: [label.name]) when Project - send("namespace_project_#{type.to_s.pluralize}_path", + send("namespace_project_#{type.to_s.pluralize}_path", # rubocop:disable GitlabSecurity/PublicSend subject.namespace, subject, label_name: [label.name]) diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb index bd75f25a210..f2707022a4b 100644 --- a/app/models/concerns/spammable.rb +++ b/app/models/concerns/spammable.rb @@ -58,7 +58,7 @@ module Spammable options.fetch(:spam_title, false) end - public_send(attr.first) if attr && respond_to?(attr.first.to_sym) + public_send(attr.first) if attr && respond_to?(attr.first.to_sym) # rubocop:disable GitlabSecurity/PublicSend end def spam_description @@ -66,12 +66,12 @@ module Spammable options.fetch(:spam_description, false) end - public_send(attr.first) if attr && respond_to?(attr.first.to_sym) + public_send(attr.first) if attr && respond_to?(attr.first.to_sym) # rubocop:disable GitlabSecurity/PublicSend end def spammable_text result = self.class.spammable_attrs.map do |attr| - public_send(attr.first) + public_send(attr.first) # rubocop:disable GitlabSecurity/PublicSend end result.reject(&:blank?).join("\n") diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 1ca7f91dc03..a7d5de48c66 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -44,7 +44,8 @@ module TokenAuthenticatable end define_method("ensure_#{token_field}!") do - send("reset_#{token_field}!") if read_attribute(token_field).blank? + send("reset_#{token_field}!") if read_attribute(token_field).blank? # rubocop:disable GitlabSecurity/PublicSend + read_attribute(token_field) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e83b11f7668..f90194041b1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -162,7 +162,7 @@ class MergeRequest < ActiveRecord::Base target = unscoped.where(target_project_id: relation).select(:id) union = Gitlab::SQL::Union.new([source, target]) - where("merge_requests.id IN (#{union.to_sql})") + where("merge_requests.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb index cafdbe11849..670b26d4ca3 100644 --- a/app/models/merge_request_diff_commit.rb +++ b/app/models/merge_request_diff_commit.rb @@ -26,7 +26,7 @@ class MergeRequestDiffCommit < ActiveRecord::Base def to_hash Gitlab::Git::Commit::SERIALIZE_KEYS.each_with_object({}) do |key, hash| - hash[key] = public_send(key) + hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index 2bc00a082df..0e5acb22d50 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -206,7 +206,7 @@ module Network # Visit branching chains leaves.each do |l| - parents = l.parents(@map).select{|p| p.space.zero?} + parents = l.parents(@map).select {|p| p.space.zero?} parents.each do |p| place_chain(p, l.time) end diff --git a/app/models/note.rb b/app/models/note.rb index 0e120e7de16..aa8e03ce302 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -77,20 +77,20 @@ class Note < ActiveRecord::Base # Scopes scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } - scope :system, ->{ where(system: true) } - scope :user, ->{ where(system: false) } - scope :common, ->{ where(noteable_type: ["", nil]) } - scope :fresh, ->{ order(created_at: :asc, id: :asc) } - scope :updated_after, ->(time){ where('updated_at > ?', time) } - scope :inc_author_project, ->{ includes(:project, :author) } - scope :inc_author, ->{ includes(:author) } + scope :system, -> { where(system: true) } + scope :user, -> { where(system: false) } + scope :common, -> { where(noteable_type: ["", nil]) } + scope :fresh, -> { order(created_at: :asc, id: :asc) } + scope :updated_after, ->(time) { where('updated_at > ?', time) } + scope :inc_author_project, -> { includes(:project, :author) } + scope :inc_author, -> { includes(:author) } scope :inc_relations_for_view, -> do includes(:project, :author, :updated_by, :resolved_by, :award_emoji, :system_note_metadata) end - scope :diff_notes, ->{ where(type: %w(LegacyDiffNote DiffNote)) } - scope :new_diff_notes, ->{ where(type: 'DiffNote') } - scope :non_diff_notes, ->{ where(type: ['Note', 'DiscussionNote', nil]) } + scope :diff_notes, -> { where(type: %w(LegacyDiffNote DiffNote)) } + scope :new_diff_notes, -> { where(type: 'DiffNote') } + scope :non_diff_notes, -> { where(type: ['Note', 'DiscussionNote', nil]) } scope :with_associations, -> do # FYI noteable cannot be loaded for LegacyDiffNote for commits diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 9b1cac64c44..245f8dddcf9 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -66,6 +66,6 @@ class NotificationSetting < ActiveRecord::Base alias_method :failed_pipeline?, :failed_pipeline def event_enabled?(event) - respond_to?(event) && !!public_send(event) + respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/project.rb b/app/models/project.rb index e7baba2ef08..7010664e1c8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -415,7 +415,7 @@ class Project < ActiveRecord::Base union = Gitlab::SQL::Union.new([projects, namespaces]) - where("projects.id IN (#{union.to_sql})") + where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end def search_by_title(query) @@ -825,7 +825,7 @@ class Project < ActiveRecord::Base if template.nil? # If no template, we should create an instance. Ex `build_gitlab_ci_service` - public_send("build_#{service_name}_service") + public_send("build_#{service_name}_service") # rubocop:disable GitlabSecurity/PublicSend else Service.build_from_template(id, template) end @@ -1046,13 +1046,18 @@ class Project < ActiveRecord::Base end def change_head(branch) - repository.before_change_head - repository.rugged.references.create('HEAD', - "refs/heads/#{branch}", - force: true) - repository.copy_gitattributes(branch) - repository.after_change_head - reload_default_branch + if repository.branch_exists?(branch) + repository.before_change_head + repository.rugged.references.create('HEAD', + "refs/heads/#{branch}", + force: true) + repository.copy_gitattributes(branch) + repository.after_change_head + reload_default_branch + else + errors.add(:base, "Could not change HEAD: branch '#{branch}' does not exist") + false + end end def forked_from?(project) @@ -1326,7 +1331,7 @@ class Project < ActiveRecord::Base end def append_or_update_attribute(name, value) - old_values = public_send(name.to_s) + old_values = public_send(name.to_s) # rubocop:disable GitlabSecurity/PublicSend if Project.reflect_on_association(name).try(:macro) == :has_many && old_values.any? update_attribute(name, old_values + value) diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index c8fabb16dc1..fb1db0255aa 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -55,7 +55,7 @@ class ProjectFeature < ActiveRecord::Base end def access_level(feature) - public_send(ProjectFeature.access_level_attribute(feature)) + public_send(ProjectFeature.access_level_attribute(feature)) # rubocop:disable GitlabSecurity/PublicSend end def builds_enabled? @@ -80,7 +80,7 @@ class ProjectFeature < ActiveRecord::Base # which cannot be higher than repository access level def repository_children_level validator = lambda do |field| - level = public_send(field) || ProjectFeature::ENABLED + level = public_send(field) || ProjectFeature::ENABLED # rubocop:disable GitlabSecurity/PublicSend not_allowed = level > repository_access_level self.errors.add(field, "cannot have higher visibility level than repository access level") if not_allowed end diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index aeaf63abab9..715b215d1db 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -14,7 +14,7 @@ class ProjectStatistics < ActiveRecord::Base def refresh!(only: nil) STATISTICS_COLUMNS.each do |column, generator| if only.blank? || only.include?(column) - public_send("update_#{column}") + public_send("update_#{column}") # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/repository.rb b/app/models/repository.rb index ff82b958255..049bebdbe42 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -300,7 +300,7 @@ class Repository expire_method_caches(to_refresh) - to_refresh.each { |method| send(method) } + to_refresh.each { |method| send(method) } # rubocop:disable GitlabSecurity/PublicSend end def expire_branch_cache(branch_name = nil) diff --git a/app/models/user.rb b/app/models/user.rb index 5148886eed7..7935b89662b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -148,6 +148,8 @@ class User < ActiveRecord::Base uniqueness: { case_sensitive: false } validate :namespace_uniq, if: :username_changed? + validate :namespace_move_dir_allowed, if: :username_changed? + validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :unique_email, if: :email_changed? validate :owns_notification_email, if: :notification_email_changed? @@ -487,6 +489,12 @@ class User < ActiveRecord::Base end end + def namespace_move_dir_allowed + if namespace&.any_project_has_container_registry_tags? + errors.add(:username, 'cannot be changed if a personal project has container registry tags.') + end + end + def avatar_type unless avatar.image? errors.add :avatar, "only images allowed" @@ -528,7 +536,7 @@ class User < ActiveRecord::Base union = Gitlab::SQL::Union .new([groups.select(:id), authorized_projects.select(:namespace_id)]) - Group.where("namespaces.id IN (#{union.to_sql})") + Group.where("namespaces.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end # Returns a relation of groups the user has access to, including their parent @@ -719,8 +727,8 @@ class User < ActiveRecord::Base def sanitize_attrs %w[username skype linkedin twitter].each do |attr| - value = public_send(attr) - public_send("#{attr}=", Sanitize.clean(value)) if value.present? + value = public_send(attr) # rubocop:disable GitlabSecurity/PublicSend + public_send("#{attr}=", Sanitize.clean(value)) if value.present? # rubocop:disable GitlabSecurity/PublicSend end end @@ -779,7 +787,7 @@ class User < ActiveRecord::Base def with_defaults User.defaults.each do |k, v| - public_send("#{k}=", v) + public_send("#{k}=", v) # rubocop:disable GitlabSecurity/PublicSend end self @@ -825,7 +833,7 @@ class User < ActiveRecord::Base { name: name, username: username, - avatar_url: avatar_url + avatar_url: avatar_url(only_path: false) } end @@ -919,7 +927,7 @@ class User < ActiveRecord::Base def ci_authorized_runners @ci_authorized_runners ||= begin runner_ids = Ci::RunnerProject - .where("ci_runner_projects.project_id IN (#{ci_projects_union.to_sql})") + .where("ci_runner_projects.project_id IN (#{ci_projects_union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection .select(:runner_id) Ci::Runner.specific.where(id: runner_ids) end diff --git a/app/serializers/analytics_build_entity.rb b/app/serializers/analytics_build_entity.rb index ad7ad020b03..bdc22d71202 100644 --- a/app/serializers/analytics_build_entity.rb +++ b/app/serializers/analytics_build_entity.rb @@ -35,6 +35,6 @@ class AnalyticsBuildEntity < Grape::Entity private def url_to(route, build, id = nil) - public_send("#{route}_url", build.project.namespace, build.project, id || build) + public_send("#{route}_url", build.project.namespace, build.project, id || build) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/serializers/analytics_issue_entity.rb b/app/serializers/analytics_issue_entity.rb index 44c50f18613..b7d95ea020f 100644 --- a/app/serializers/analytics_issue_entity.rb +++ b/app/serializers/analytics_issue_entity.rb @@ -24,6 +24,6 @@ class AnalyticsIssueEntity < Grape::Entity private def url_to(route, id) - public_send("#{route}_url", request.project.namespace, request.project, id) + public_send("#{route}_url", request.project.namespace, request.project, id) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/serializers/job_entity.rb b/app/serializers/job_entity.rb index d6de43bcbcb..72e56a2c77f 100644 --- a/app/serializers/job_entity.rb +++ b/app/serializers/job_entity.rb @@ -46,6 +46,6 @@ class JobEntity < Grape::Entity end def path_to(route, build) - send("#{route}_path", build.project.namespace, build.project, build) + send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 9114f0ccc81..234fcbede03 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -16,6 +16,7 @@ module Issues spam_check(issue, current_user) issue.move_to_end + # current_user (defined in BaseService) is not available within run_after_commit block user = current_user issue.run_after_commit do NewIssueWorker.perform_async(issue.id, user.id) diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb index d2ece354efc..775efed48eb 100644 --- a/app/services/labels/transfer_service.rb +++ b/app/services/labels/transfer_service.rb @@ -37,7 +37,7 @@ module Labels union = Gitlab::SQL::Union.new(label_ids) - Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq + Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq # rubocop:disable GitlabSecurity/SqlInjection end def group_labels_applied_to_issues diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 7d539fa49e6..fa0c0b7175c 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -17,6 +17,7 @@ module MergeRequests end def before_create(merge_request) + # current_user (defined in BaseService) is not available within run_after_commit block user = current_user merge_request.run_after_commit do NewMergeRequestWorker.perform_async(merge_request.id, user.id) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d81035e4eba..cf69007bc3b 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -10,7 +10,7 @@ module Projects end if changing_default_branch? - project.change_head(params[:default_branch]) + return error("Could not set the default branch") unless project.change_head(params[:default_branch]) end if project.update_attributes(params.except(:default_branch)) diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index df1dc736571..b32cfe158bb 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -74,7 +74,8 @@ = link_to "Profile", current_user, class: 'profile-link', data: { user: current_user.username } %li = link_to "Settings", profile_path - = render 'shared/user_dropdown_experimental_features' + %li + = link_to "Turn on new navigation", profile_preferences_path(anchor: "new-navigation") %li.divider %li = link_to "Sign out", destroy_user_session_path, method: :delete, class: "sign-out-link" diff --git a/app/views/layouts/header/_new.html.haml b/app/views/layouts/header/_new.html.haml index fa94925d537..2c1c23d6ea9 100644 --- a/app/views/layouts/header/_new.html.haml +++ b/app/views/layouts/header/_new.html.haml @@ -68,7 +68,8 @@ = link_to "Profile", current_user, class: 'profile-link', data: { user: current_user.username } %li = link_to "Settings", profile_path - = render 'shared/user_dropdown_experimental_features' + %li + = link_to "Turn off new navigation", profile_preferences_path(anchor: "new-navigation") %li.divider %li = link_to "Sign out", destroy_user_session_path, method: :delete, class: "sign-out-link" diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 9bd8bf91d1c..f08dcc0c242 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -18,8 +18,6 @@ = scheme.name .col-sm-12 %hr - %h3#experimental-features Experimental features - %hr .col-lg-4.profile-settings-sidebar#new-navigation %h4.prepend-top-0 New Navigation @@ -42,28 +40,6 @@ New .col-sm-12 %hr - .col-lg-4.profile-settings-sidebar#new-repository - %h4.prepend-top-0 - New Repository - %p - This setting allows you to turn on or off the new upcoming repository concept. - .col-lg-8.syntax-theme - .nav-wip - %p - The new repository is currently a work-in-progress concept and only usable on wide-screens. There are a number of improvements that we are working on in order to further refine the repository view. - %p - %a{ href: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/31890', target: 'blank' } Learn more - about the improvements that are coming soon! - = label_tag do - .preview= image_tag "old_repo.png" - %input.js-experiment-feature-toggle{ type: "radio", value: "false", name: "new_repo", checked: !show_new_repo? } - Old - = label_tag do - .preview= image_tag "new_repo.png" - %input.js-experiment-feature-toggle{ type: "radio", value: "true", name: "new_repo", checked: show_new_repo? } - New - .col-sm-12 - %hr .col-lg-4.profile-settings-sidebar %h4.prepend-top-0 Behavior diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 8c8aa4c78f5..178ab3df2e5 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -2,22 +2,24 @@ - show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true) - can_create_note = !@diff_notes_disabled && can?(current_user, :create_note, diffs.project) - diff_files = diffs.diff_files +- merge_request = local_assigns.fetch(:merge_request, false) -.content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed - .inline-parallel-buttons - - if !diffs_expanded? && diff_files.any? { |diff_file| diff_file.collapsed? } - = link_to 'Expand all', url_for(params.merge(expanded: 1, format: nil)), class: 'btn btn-default' - - if show_whitespace_toggle - - if current_controller?(:commit) - = commit_diff_whitespace_link(diffs.project, @commit, class: 'hidden-xs') - - elsif current_controller?('projects/merge_requests/diffs') - = diff_merge_request_whitespace_link(diffs.project, @merge_request, class: 'hidden-xs') - - elsif current_controller?(:compare) - = diff_compare_whitespace_link(diffs.project, params[:from], params[:to], class: 'hidden-xs') - .btn-group - = inline_diff_btn - = parallel_diff_btn - = render 'projects/diffs/stats', diff_files: diff_files +.content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed{ class: ("diff-files-changed-merge-request" if merge_request) } + .files-changed-inner + .inline-parallel-buttons + - if !diffs_expanded? && diff_files.any? { |diff_file| diff_file.collapsed? } + = link_to 'Expand all', url_for(params.merge(expanded: 1, format: nil)), class: 'btn btn-default' + - if show_whitespace_toggle + - if current_controller?(:commit) + = commit_diff_whitespace_link(diffs.project, @commit, class: 'hidden-xs') + - elsif current_controller?('projects/merge_requests/diffs') + = diff_merge_request_whitespace_link(diffs.project, @merge_request, class: 'hidden-xs') + - elsif current_controller?(:compare) + = diff_compare_whitespace_link(diffs.project, params[:from], params[:to], class: 'hidden-xs') + .btn-group + = inline_diff_btn + = parallel_diff_btn + = render 'projects/diffs/stats', diff_files: diff_files - if render_overflow_warning?(diff_files) = render 'projects/diffs/warning', diff_files: diffs diff --git a/app/views/projects/merge_requests/diffs/_diffs.html.haml b/app/views/projects/merge_requests/diffs/_diffs.html.haml index fb31e2fef00..0d30d6da68f 100644 --- a/app/views/projects/merge_requests/diffs/_diffs.html.haml +++ b/app/views/projects/merge_requests/diffs/_diffs.html.haml @@ -1,5 +1,5 @@ - if @merge_request_diff.collected? || @merge_request_diff.overflow? = render 'projects/merge_requests/diffs/versions' - = render "projects/diffs/diffs", diffs: @diffs, environment: @environment + = render "projects/diffs/diffs", diffs: @diffs, environment: @environment, merge_request: true - elsif @merge_request_diff.empty? .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} diff --git a/app/views/shared/_user_dropdown_experimental_features.html.haml b/app/views/shared/_user_dropdown_experimental_features.html.haml deleted file mode 100644 index 8e71407b748..00000000000 --- a/app/views/shared/_user_dropdown_experimental_features.html.haml +++ /dev/null @@ -1 +0,0 @@ -%li= link_to 'Experimental features', profile_preferences_path(anchor: 'experimental-features') diff --git a/app/workers/concerns/new_issuable.rb b/app/workers/concerns/new_issuable.rb index 3fd472bf0c1..eb0d6c9c36c 100644 --- a/app/workers/concerns/new_issuable.rb +++ b/app/workers/concerns/new_issuable.rb @@ -1,20 +1,23 @@ module NewIssuable attr_reader :issuable, :user - def ensure_objects_found(issuable_id, user_id) - @issuable = issuable_class.find_by(id: issuable_id) - unless @issuable - log_error(issuable_class, issuable_id) - return false - end + def objects_found?(issuable_id, user_id) + set_user(user_id) + set_issuable(issuable_id) + + user && issuable + end + def set_user(user_id) @user = User.find_by(id: user_id) - unless @user - log_error(User, user_id) - return false - end - true + log_error(User, user_id) unless @user + end + + def set_issuable(issuable_id) + @issuable = issuable_class.find_by(id: issuable_id) + + log_error(issuable_class, issuable_id) unless @issuable end def log_error(record_class, record_id) diff --git a/app/workers/new_issue_worker.rb b/app/workers/new_issue_worker.rb index 19a778ad522..d9a8e892e90 100644 --- a/app/workers/new_issue_worker.rb +++ b/app/workers/new_issue_worker.rb @@ -4,7 +4,7 @@ class NewIssueWorker include NewIssuable def perform(issue_id, user_id) - return unless ensure_objects_found(issue_id, user_id) + return unless objects_found?(issue_id, user_id) EventCreateService.new.open_issue(issuable, user) NotificationService.new.new_issue(issuable, user) diff --git a/app/workers/new_merge_request_worker.rb b/app/workers/new_merge_request_worker.rb index 3c8a68016ff..1910c490159 100644 --- a/app/workers/new_merge_request_worker.rb +++ b/app/workers/new_merge_request_worker.rb @@ -4,7 +4,7 @@ class NewMergeRequestWorker include NewIssuable def perform(merge_request_id, user_id) - return unless ensure_objects_found(merge_request_id, user_id) + return unless objects_found?(merge_request_id, user_id) EventCreateService.new.open_mr(issuable, user) NotificationService.new.new_merge_request(issuable, user) diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb index 4eeb9666bb0..64788da7299 100644 --- a/app/workers/pages_worker.rb +++ b/app/workers/pages_worker.rb @@ -4,7 +4,7 @@ class PagesWorker sidekiq_options queue: :pages, retry: false def perform(action, *arg) - send(action, *arg) + send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend end def deploy(build_id) diff --git a/bin/rspec-stackprof b/bin/rspec-stackprof index df79feb201d..810863ea4a0 100755 --- a/bin/rspec-stackprof +++ b/bin/rspec-stackprof @@ -1,5 +1,6 @@ #!/usr/bin/env ruby +require 'bundler/setup' require 'stackprof' $:.unshift 'spec' require 'rails_helper' @@ -13,4 +14,4 @@ StackProf.run(mode: :wall, out: output_file, interval: interval) do RSpec::Core::Runner.run(ARGV, $stderr, $stdout) end -system("stackprof #{output_file} --text --limit #{limit}") +system("bundle exec stackprof #{output_file} --text --limit #{limit}") diff --git a/changelogs/unreleased/34339-user_avatar-url-in-push-event-webhook-json-payload-is-relative-should-be-absolute.yml b/changelogs/unreleased/34339-user_avatar-url-in-push-event-webhook-json-payload-is-relative-should-be-absolute.yml new file mode 100644 index 00000000000..13f28da8577 --- /dev/null +++ b/changelogs/unreleased/34339-user_avatar-url-in-push-event-webhook-json-payload-is-relative-should-be-absolute.yml @@ -0,0 +1,4 @@ +--- +title: Use full path of user's avatar in webhooks +merge_request: 13401 +author: Vitaliy @blackst0ne Klachkov diff --git a/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml new file mode 100644 index 00000000000..04791e09b84 --- /dev/null +++ b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml @@ -0,0 +1,4 @@ +--- +title: Add checks for branch existence before changing HEAD +merge_request: 13359 +author: Vitaliy @blackst0ne Klachkov diff --git a/changelogs/unreleased/36119-issuable-workers.yml b/changelogs/unreleased/36119-issuable-workers.yml new file mode 100644 index 00000000000..beb01ae5b1a --- /dev/null +++ b/changelogs/unreleased/36119-issuable-workers.yml @@ -0,0 +1,4 @@ +--- +title: Simplify checking if objects exist code in new issaubles workers +merge_request: +author: diff --git a/changelogs/unreleased/mk-validate-username-change-with-container-registry-tags.yml b/changelogs/unreleased/mk-validate-username-change-with-container-registry-tags.yml new file mode 100644 index 00000000000..425d5231e14 --- /dev/null +++ b/changelogs/unreleased/mk-validate-username-change-with-container-registry-tags.yml @@ -0,0 +1,4 @@ +--- +title: Add missing validation error for username change with container registry tags +merge_request: 13356 +author: diff --git a/config/application.rb b/config/application.rb index 47887bf8596..f69dab4de39 100644 --- a/config/application.rb +++ b/config/application.rb @@ -176,7 +176,7 @@ module Gitlab next unless name.include?('namespace_project') define_method(name.sub('namespace_project', 'project')) do |project, *args| - send(name, project&.namespace, project, *args) + send(name, project&.namespace, project, *args) # rubocop:disable GitlabSecurity/PublicSend end end end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 2699173fc61..5c6578d3531 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -71,7 +71,7 @@ class Settings < Settingslogic # check that `current` (string or integer) is a contant in `modul`. def verify_constant(modul, current, default) - constant = modul.constants.find{ |name| modul.const_get(name) == current } + constant = modul.constants.find { |name| modul.const_get(name) == current } value = constant.nil? ? default : modul.const_get(constant) if current.is_a? String value = modul.const_get(current.upcase) rescue default diff --git a/config/initializers/active_record_locking.rb b/config/initializers/active_record_locking.rb index 9266ff0f615..150aaa2a8c2 100644 --- a/config/initializers/active_record_locking.rb +++ b/config/initializers/active_record_locking.rb @@ -18,7 +18,7 @@ module ActiveRecord lock_col = self.class.locking_column - previous_lock_value = send(lock_col).to_i + previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend # This line is added as a patch previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0 @@ -48,7 +48,7 @@ module ActiveRecord # If something went wrong, revert the version. rescue Exception - send(lock_col + '=', previous_lock_value) + send(lock_col + '=', previous_lock_value) # rubocop:disable GitlabSecurity/PublicSend raise end end diff --git a/config/routes/repository.rb b/config/routes/repository.rb index 2ba16035ece..57b7c55423d 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -3,6 +3,9 @@ resource :repository, only: [:create] do member do get ':ref/archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex, ref: /.+/ }, action: 'archive', as: 'archive' + + # deprecated since GitLab 9.5 + get 'archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex }, as: 'archive_alternative' end end diff --git a/db/migrate/20161017125927_add_unique_index_to_labels.rb b/db/migrate/20161017125927_add_unique_index_to_labels.rb index b8f6a803a0a..fcdd79d3b02 100644 --- a/db/migrate/20161017125927_add_unique_index_to_labels.rb +++ b/db/migrate/20161017125927_add_unique_index_to_labels.rb @@ -10,7 +10,7 @@ class AddUniqueIndexToLabels < ActiveRecord::Migration def up select_all('SELECT title, project_id, COUNT(id) as cnt FROM labels GROUP BY project_id, title HAVING COUNT(id) > 1').each do |label| label_title = quote_string(label['title']) - duplicated_ids = select_all("SELECT id FROM labels WHERE project_id = #{label['project_id']} AND title = '#{label_title}' ORDER BY id ASC").map{ |label| label['id'] } + duplicated_ids = select_all("SELECT id FROM labels WHERE project_id = #{label['project_id']} AND title = '#{label_title}' ORDER BY id ASC").map { |label| label['id'] } label_id = duplicated_ids.first duplicated_ids.delete(label_id) diff --git a/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb b/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb index c1e64f20109..5238a2ba1b7 100644 --- a/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb +++ b/db/post_migrate/20170502101023_cleanup_namespaceless_pending_delete_projects.rb @@ -30,7 +30,7 @@ class CleanupNamespacelessPendingDeleteProjects < ActiveRecord::Migration private def pending_delete_batch - connection.exec_query(find_batch).map{ |row| row['id'].to_i } + connection.exec_query(find_batch).map { |row| row['id'].to_i } end BATCH_SIZE = 5000 diff --git a/doc/README.md b/doc/README.md index 8bb8e147cd1..ca4790ceda0 100644 --- a/doc/README.md +++ b/doc/README.md @@ -3,7 +3,7 @@ Welcome to [GitLab](https://about.gitlab.com/), a Git-based fully featured platform for software development! -We offer four different products for you and your company: +GitLab offers the most scalable Git-based fully integrated platform for software development, with flexible products and subscription plans: - **GitLab Community Edition (CE)** is an [opensource product](https://gitlab.com/gitlab-org/gitlab-ce/), self-hosted, free to use. Every feature available in GitLab CE is also available on GitLab Enterprise Edition (Starter and Premium) and GitLab.com. diff --git a/doc/administration/logs.md b/doc/administration/logs.md index 4b8d5c5cc87..76e071dc673 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -9,6 +9,33 @@ documentation](http://docs.gitlab.com/ee/administration/audit_events.html) System log files are typically plain text in a standard log file format. This guide talks about how to read and use these system log files. +## `production_json.log` + +This file lives in `/var/log/gitlab/gitlab-rails/production_json.log` for +Omnibus GitLab packages or in `/home/git/gitlab/log/production_json.log` for +installations from source. (When Gitlab is running in an environment +other than production, the corresponding logfile is shown here.) + +It contains a structured log for Rails controller requests received from +GitLab, thanks to [Lograge](https://github.com/roidrage/lograge/). Note that +requests from the API [are not yet logged to this +file](https://gitlab.com/gitlab-org/gitlab-ce/issues/36189). + +Each line contains a JSON line that can be ingested by Elasticsearch, Splunk, etc. For example: + +```json +{"method":"GET","path":"/gitlab/gitlab-ce/issues/1234","format":"html","controller":"Projects::IssuesController","action":"show","status":200,"duration":229.03,"view":174.07,"db":13.24,"time":"2017-08-08T20:15:54.821Z","params":{"namespace_id":"gitlab","project_id":"gitlab-ce","id":"1234"},"remote_ip":"18.245.0.1","user_id":1,"username":"admin"} +``` + +In this example, you can see this was a GET request for a specific issue. Notice each line also contains performance data: + +1. `duration`: the total time taken to retrieve the request +2. `view`: total time taken inside the Rails views +3. `db`: total time to retrieve data from the database + +In addition, the log contains the IP address from which the request originated +(`remote_ip`) as well as the user's ID (`user_id`), and username (`username`). + ## `production.log` This file lives in `/var/log/gitlab/gitlab-rails/production.log` for diff --git a/doc/install/installation.md b/doc/install/installation.md index 22aedb5403e..e335fc99fbf 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -296,9 +296,9 @@ sudo usermod -aG redis git ### Clone the Source # Clone GitLab repository - sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 9-4-stable gitlab + sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 9-5-stable gitlab -**Note:** You can change `9-4-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! +**Note:** You can change `9-5-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! ### Configure It @@ -507,15 +507,17 @@ Check if GitLab and its environment are configured correctly: sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production + +### Compile GetText PO files + + sudo -u git -H bundle exec rake gettext:pack RAILS_ENV=production + sudo -u git -H bundle exec rake gettext:po_to_json RAILS_ENV=production + ### Compile Assets sudo -u git -H yarn install --production --pure-lockfile sudo -u git -H bundle exec rake gitlab:assets:compile RAILS_ENV=production NODE_ENV=production -### Compile GetText PO files - - sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production - ### Start Your GitLab Instance sudo service gitlab start diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md index 583ec5522fd..0399ebec86a 100644 --- a/doc/system_hooks/system_hooks.md +++ b/doc/system_hooks/system_hooks.md @@ -31,7 +31,7 @@ X-Gitlab-Event: System Hook "path": "storecloud", "path_with_namespace": "jsmith/storecloud", "project_id": 74, - "project_visibility": "private", + "project_visibility": "private" } ``` @@ -48,7 +48,7 @@ X-Gitlab-Event: System Hook "path": "underscore", "path_with_namespace": "jsmith/underscore", "project_id": 73, - "project_visibility": "internal", + "project_visibility": "internal" } ``` @@ -66,7 +66,7 @@ X-Gitlab-Event: System Hook "owner_name": "John Smith", "owner_email": "johnsmith@gmail.com", "project_visibility": "internal", - "old_path_with_namespace": "jsmith/overscore", + "old_path_with_namespace": "jsmith/overscore" } ``` @@ -84,7 +84,7 @@ X-Gitlab-Event: System Hook "owner_name": "John Smith", "owner_email": "johnsmith@gmail.com", "project_visibility": "internal", - "old_path_with_namespace": "jsmith/overscore", + "old_path_with_namespace": "jsmith/overscore" } ``` @@ -101,7 +101,7 @@ X-Gitlab-Event: System Hook "path": "storecloud", "path_with_namespace": "jsmith/storecloud", "project_id": 74, - "project_visibility": "private", + "project_visibility": "private" } ``` @@ -121,7 +121,7 @@ X-Gitlab-Event: System Hook "user_name": "John Smith", "user_username": "johnsmith", "user_id": 41, - "project_visibility": "private", + "project_visibility": "private" } ``` @@ -141,7 +141,7 @@ X-Gitlab-Event: System Hook "user_name": "John Smith", "user_username": "johnsmith", "user_id": 41, - "project_visibility": "private", + "project_visibility": "private" } ``` diff --git a/doc/update/9.1-to-9.2.md b/doc/update/9.1-to-9.2.md index f38547bba1a..6f19d16ad74 100644 --- a/doc/update/9.1-to-9.2.md +++ b/doc/update/9.1-to-9.2.md @@ -100,6 +100,7 @@ cd /home/git/gitlab sudo -u git -H git fetch --all sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +sudo -u git -H git checkout -- locale ``` For GitLab Community Edition: @@ -236,6 +237,11 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:pack RAILS_ENV=production +sudo -u git -H bundle exec rake gettext:po_to_json RAILS_ENV=production + # Update node dependencies and recompile assets sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production diff --git a/doc/update/9.2-to-9.3.md b/doc/update/9.2-to-9.3.md index 373f43eb3bb..9415fa1fcd6 100644 --- a/doc/update/9.2-to-9.3.md +++ b/doc/update/9.2-to-9.3.md @@ -100,6 +100,7 @@ cd /home/git/gitlab sudo -u git -H git fetch --all sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +sudo -u git -H git checkout -- locale ``` For GitLab Community Edition: @@ -272,6 +273,10 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production + # Update node dependencies and recompile assets sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production diff --git a/doc/update/9.3-to-9.4.md b/doc/update/9.3-to-9.4.md index b167f0737aa..982385f3c24 100644 --- a/doc/update/9.3-to-9.4.md +++ b/doc/update/9.3-to-9.4.md @@ -100,6 +100,7 @@ cd /home/git/gitlab sudo -u git -H git fetch --all sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +sudo -u git -H git checkout -- locale ``` For GitLab Community Edition: @@ -285,6 +286,10 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production + # Update node dependencies and recompile assets sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production diff --git a/doc/update/9.4-to-9.5.md b/doc/update/9.4-to-9.5.md index fc87b2d0f1e..1b5a15589af 100644 --- a/doc/update/9.4-to-9.5.md +++ b/doc/update/9.4-to-9.5.md @@ -295,6 +295,10 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production + # Update node dependencies and recompile assets sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production diff --git a/doc/update/patch_versions.md b/doc/update/patch_versions.md index ac1bcb8f241..12408123158 100644 --- a/doc/update/patch_versions.md +++ b/doc/update/patch_versions.md @@ -35,7 +35,7 @@ current version with `cat VERSION`). cd /home/git/gitlab sudo -u git -H git fetch --all -sudo -u git -H git checkout -- Gemfile.lock db/schema.rb +sudo -u git -H git checkout -- Gemfile.lock db/schema.rb locale sudo -u git -H git checkout LATEST_TAG -b LATEST_TAG ``` @@ -56,11 +56,21 @@ sudo -u git -H bundle clean # Run database migrations sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production +### 4. Compile GetText PO files + +Internationalization was added in `v9.2.0` so these commands are only +required for versions equal or major to it. + +```bash +sudo -u git -H bundle exec rake gettext:pack RAILS_ENV=production +sudo -u git -H bundle exec rake gettext:po_to_json RAILS_ENV=production +``` + # Clean up assets and cache sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile cache:clear RAILS_ENV=production NODE_ENV=production ``` -### 4. Update gitlab-workhorse to the corresponding version +### 5. Update gitlab-workhorse to the corresponding version ```bash cd /home/git/gitlab @@ -68,7 +78,7 @@ cd /home/git/gitlab sudo -u git -H bundle exec rake "gitlab:workhorse:install[/home/git/gitlab-workhorse]" RAILS_ENV=production ``` -### 5. Update gitlab-shell to the corresponding version +### 6. Update gitlab-shell to the corresponding version ```bash cd /home/git/gitlab-shell @@ -78,14 +88,14 @@ sudo -u git -H git checkout v`cat /home/git/gitlab/GITLAB_SHELL_VERSION` -b v`ca sudo -u git -H sh -c 'if [ -x bin/compile ]; then bin/compile; fi' ``` -### 6. Start application +### 7. Start application ```bash sudo service gitlab start sudo service nginx restart ``` -### 7. Check application status +### 8. Check application status Check if GitLab and its environment are configured correctly: diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 225879d94ac..7ee25bf177c 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -132,7 +132,7 @@ module API expose :lfs_enabled?, as: :lfs_enabled expose :creator_id expose :namespace, using: 'API::Entities::Namespace' - expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda{ |project, options| project.forked? } + expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } expose :avatar_url do |user, options| diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb index d9cae1501f8..a50ea0b52aa 100644 --- a/lib/api/helpers/members_helpers.rb +++ b/lib/api/helpers/members_helpers.rb @@ -1,8 +1,10 @@ +# rubocop:disable GitlabSecurity/PublicSend + module API module Helpers module MembersHelpers def find_source(source_type, id) - public_send("find_#{source_type}!", id) + public_send("find_#{source_type}!", id) # rubocop:disable GitlabSecurity/PublicSend end def authorize_admin_source!(source_type, source) diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 65ff89edf65..4e4e473994b 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -139,7 +139,7 @@ module API helpers do def find_project_noteable(noteables_str, noteable_id) - public_send("find_project_#{noteables_str.singularize}", noteable_id) + public_send("find_project_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend end def noteable_read_ability_name(noteable) diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 773f667abe0..4a2e9c9cbb0 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -68,7 +68,7 @@ module API expose :lfs_enabled?, as: :lfs_enabled expose :creator_id expose :namespace, using: 'API::Entities::Namespace' - expose :forked_from_project, using: ::API::Entities::BasicProjectDetails, if: lambda{ |project, options| project.forked? } + expose :forked_from_project, using: ::API::Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :avatar_url do |user, options| user.avatar_url(only_path: false) end diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index ca6d6848d41..b9a573d3542 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -198,11 +198,11 @@ module Backup end def archives_to_backup - ARCHIVES_TO_BACKUP.map{ |name| (name + ".tar.gz") unless skipped?(name) }.compact + ARCHIVES_TO_BACKUP.map { |name| (name + ".tar.gz") unless skipped?(name) }.compact end def folders_to_backup - FOLDERS_TO_BACKUP.reject{ |name| skipped?(name) } + FOLDERS_TO_BACKUP.reject { |name| skipped?(name) } end def disabled_features diff --git a/lib/ci/ansi2html.rb b/lib/ci/ansi2html.rb index 55402101e43..8354fc8d595 100644 --- a/lib/ci/ansi2html.rb +++ b/lib/ci/ansi2html.rb @@ -254,7 +254,7 @@ module Ci def state state = STATE_PARAMS.inject({}) do |h, param| - h[param] = send(param) + h[param] = send(param) # rubocop:disable GitlabSecurity/PublicSend h end Base64.urlsafe_encode64(state.to_json) @@ -266,7 +266,7 @@ module Ci return if state[:offset].to_i > stream.size STATE_PARAMS.each do |param| - send("#{param}=".to_sym, state[param]) + send("#{param}=".to_sym, state[param]) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/lib/ci/charts.rb b/lib/ci/charts.rb index 872e418c788..76a69bf8a83 100644 --- a/lib/ci/charts.rb +++ b/lib/ci/charts.rb @@ -47,7 +47,7 @@ module Ci def collect query = project.pipelines - .where("? > #{Ci::Pipeline.table_name}.created_at AND #{Ci::Pipeline.table_name}.created_at > ?", @to, @from) + .where("? > #{Ci::Pipeline.table_name}.created_at AND #{Ci::Pipeline.table_name}.created_at > ?", @to, @from) # rubocop:disable GitlabSecurity/SqlInjection totals_count = grouped_count(query) success_count = grouped_count(query.success) diff --git a/lib/constraints/project_url_constrainer.rb b/lib/constraints/project_url_constrainer.rb index 4c0aee6c48f..fd7b97d3167 100644 --- a/lib/constraints/project_url_constrainer.rb +++ b/lib/constraints/project_url_constrainer.rb @@ -6,6 +6,8 @@ class ProjectUrlConstrainer return false unless DynamicPathValidator.valid_project_path?(full_path) + # We intentionally allow SELECT(*) here so result of this query can be used + # as cache for further Project.find_by_full_path calls within request Project.find_by_full_path(full_path, follow_redirects: request.get?).present? end end diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 2d89ccfc354..0603141e441 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -21,7 +21,7 @@ module Gitlab def to_hash hash = {} - serialize_keys.each { |key| hash[key] = send(key) } + serialize_keys.each { |key| hash[key] = send(key) } # rubocop:disable GitlabSecurity/PublicSend hash end diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 72d7d4f84d1..abd401224d8 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -98,10 +98,11 @@ module Gitlab if status.zero? @ee_branch_found = ee_branch_prefix - else - _, status = step("Fetching origin/#{ee_branch_suffix}", %W[git fetch origin #{ee_branch_suffix}]) + return end + _, status = step("Fetching origin/#{ee_branch_suffix}", %W[git fetch origin #{ee_branch_suffix}]) + if status.zero? @ee_branch_found = ee_branch_suffix else diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 9256663f454..fd4dfdb09a2 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -319,7 +319,7 @@ module Gitlab def to_hash serialize_keys.map.with_object({}) do |key, hash| - hash[key] = send(key) + hash[key] = send(key) # rubocop:disable GitlabSecurity/PublicSend end end @@ -412,7 +412,7 @@ module Gitlab raw_commit = hash.symbolize_keys serialize_keys.each do |key| - send("#{key}=", raw_commit[key]) + send("#{key}=", raw_commit[key]) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index 9e00abefd02..ce3d65062e8 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -143,7 +143,7 @@ module Gitlab hash = {} SERIALIZE_KEYS.each do |key| - hash[key] = send(key) + hash[key] = send(key) # rubocop:disable GitlabSecurity/PublicSend end hash @@ -221,7 +221,7 @@ module Gitlab raw_diff = hash.symbolize_keys SERIALIZE_KEYS.each do |key| - send(:"#{key}=", raw_diff[key.to_sym]) + send(:"#{key}=", raw_diff[key.to_sym]) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/lib/gitlab/gitaly_client/diff.rb b/lib/gitlab/gitaly_client/diff.rb index d459c9a88fb..54df6304865 100644 --- a/lib/gitlab/gitaly_client/diff.rb +++ b/lib/gitlab/gitaly_client/diff.rb @@ -7,13 +7,13 @@ module Gitlab def initialize(params) params.each do |key, val| - public_send(:"#{key}=", val) + public_send(:"#{key}=", val) # rubocop:disable GitlabSecurity/PublicSend end end def ==(other) FIELDS.all? do |field| - public_send(field) == other.public_send(field) + public_send(field) == other.public_send(field) # rubocop:disable GitlabSecurity/PublicSend end end end diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index 79ce784f2f2..6ad97e62941 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -10,7 +10,7 @@ module Gitlab def exists? request = Gitaly::RepositoryExistsRequest.new(repository: @gitaly_repo) - GitalyClient.call(@storage, :repository_service, :exists, request).exists + GitalyClient.call(@storage, :repository_service, :repository_exists, request).exists end def garbage_collect(create_bitmap) diff --git a/lib/gitlab/gitlab_import/client.rb b/lib/gitlab/gitlab_import/client.rb index 86fb6c51765..f1007daab5d 100644 --- a/lib/gitlab/gitlab_import/client.rb +++ b/lib/gitlab/gitlab_import/client.rb @@ -71,7 +71,7 @@ module Gitlab end def config - Gitlab.config.omniauth.providers.find{|provider| provider.name == "gitlab"} + Gitlab.config.omniauth.providers.find {|provider| provider.name == "gitlab"} end def gitlab_options diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index f5b757ace77..bc836dcc08d 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -45,7 +45,7 @@ module Gitlab end def all - REFERABLES.each { |referable| send(referable.to_s.pluralize) } + REFERABLES.each { |referable| send(referable.to_s.pluralize) } # rubocop:disable GitlabSecurity/PublicSend @references.values.flatten end diff --git a/lib/static_model.rb b/lib/static_model.rb index 185921d8fbe..60e2dd82e4e 100644 --- a/lib/static_model.rb +++ b/lib/static_model.rb @@ -18,7 +18,7 @@ module StaticModel # # Pass it along if we respond to it. def [](key) - send(key) if respond_to?(key) + send(key) if respond_to?(key) # rubocop:disable GitlabSecurity/PublicSend end def to_param diff --git a/lib/support/nginx/gitlab-pages b/lib/support/nginx/gitlab-pages index d9746c5c1aa..875c8bcbf3c 100644 --- a/lib/support/nginx/gitlab-pages +++ b/lib/support/nginx/gitlab-pages @@ -18,8 +18,11 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; + + proxy_cache off; + # The same address as passed to GitLab Pages: `-listen-proxy` - proxy_pass http://localhost:8090/; + proxy_pass http://localhost:8090/; } # Define custom error pages diff --git a/lib/support/nginx/gitlab-pages-ssl b/lib/support/nginx/gitlab-pages-ssl index a1ccf266835..62ed482e2bf 100644 --- a/lib/support/nginx/gitlab-pages-ssl +++ b/lib/support/nginx/gitlab-pages-ssl @@ -67,8 +67,11 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; + + proxy_cache off; + # The same address as passed to GitLab Pages: `-listen-proxy` - proxy_pass http://localhost:8090/; + proxy_pass http://localhost:8090/; } # Define custom error pages diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index dbb3b827b9a..1bd36bbe20a 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -41,8 +41,6 @@ namespace :gitlab do end namespace :gitlab_shell do - include SystemCheck::Helpers - desc "GitLab | Check the configuration of GitLab Shell" task check: :environment do warn_user_is_not_gitlab @@ -249,8 +247,6 @@ namespace :gitlab do end namespace :sidekiq do - include SystemCheck::Helpers - desc "GitLab | Check the configuration of Sidekiq" task check: :environment do warn_user_is_not_gitlab @@ -309,8 +305,6 @@ namespace :gitlab do end namespace :incoming_email do - include SystemCheck::Helpers - desc "GitLab | Check the configuration of Reply by email" task check: :environment do warn_user_is_not_gitlab @@ -444,8 +438,6 @@ namespace :gitlab do end namespace :ldap do - include SystemCheck::Helpers - task :check, [:limit] => :environment do |_, args| # Only show up to 100 results because LDAP directories can be very big. # This setting only affects the `rake gitlab:check` script. @@ -501,8 +493,6 @@ namespace :gitlab do end namespace :repo do - include SystemCheck::Helpers - desc "GitLab | Check the integrity of the repositories managed by GitLab" task check: :environment do Gitlab.config.repositories.storages.each do |name, repository_storage| @@ -517,8 +507,6 @@ namespace :gitlab do end namespace :user do - include SystemCheck::Helpers - desc "GitLab | Check the integrity of a specific user's repositories" task :check_repos, [:username] => :environment do |t, args| username = args[:username] || prompt("Check repository integrity for fsername? ".color(:blue)) diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index aaf00bd703a..1f504485e4c 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -21,7 +21,7 @@ namespace :gitlab do create_gitaly_configuration # In CI we run scripts/gitaly-test-build instead of this command unless ENV['CI'].present? - Bundler.with_original_env { run_command!(%w[/usr/bin/env -u RUBYOPT] + [command]) } + Bundler.with_original_env { run_command!(%w[/usr/bin/env -u RUBYOPT -u BUNDLE_GEMFILE] + [command]) } end end end diff --git a/lib/tasks/gitlab/helpers.rake b/lib/tasks/gitlab/helpers.rake index dd2d5861481..b0a24790c4a 100644 --- a/lib/tasks/gitlab/helpers.rake +++ b/lib/tasks/gitlab/helpers.rake @@ -4,5 +4,5 @@ require 'tasks/gitlab/task_helpers' StateMachines::Machine.ignore_method_conflicts = true if ENV['CRON'] namespace :gitlab do - include Gitlab::TaskHelpers + extend SystemCheck::Helpers end diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index 28b2d86eed2..d85b810ac66 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -5,6 +5,8 @@ module Gitlab TaskAbortedByUserError = Class.new(StandardError) module TaskHelpers + extend self + # Ask if the user wants to continue # # Returns "yes" the user chose to continue diff --git a/locale/ko/gitlab.po b/locale/ko/gitlab.po index 97a844ada7f..0a6fbac0880 100644 --- a/locale/ko/gitlab.po +++ b/locale/ko/gitlab.po @@ -1,4 +1,7 @@ -# chang-ho,cha <changho.cha@gmail.com>, 2017. #zanata +# Korean translations for gitlab package. +# Copyright (C) 2017 THE PACKAGE'S COPYRIGHT HOLDER +# This file is distributed under the same license as the gitlab package. +# FIRST AUTHOR <EMAIL@ADDRESS>, 2017. # Huang Tao <htve@outlook.com>, 2017. #zanata msgid "" msgstr "" @@ -8,12 +11,12 @@ msgstr "" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -"PO-Revision-Date: 2017-08-06 09:40-0400\n" +"PO-Revision-Date: 2017-08-08 08:32-0400\n" "Last-Translator: chang-ho,cha <changho.cha@gmail.com>\n" "Language-Team: Korean (https://translate.zanata.org/project/view/GitLab)\n" "Language: ko\n" +"Plural-Forms: nplurals=1; plural=0;\n" "X-Generator: Zanata 3.9.6\n" -"Plural-Forms: nplurals=1; plural=0\n" msgid "%d commit" msgid_plural "%d commits" @@ -25,7 +28,7 @@ msgid_plural "" msgstr[0] "%s 추가 커밋은 성능 이슈를 방지하기 위해 생략되었습니다." msgid "%{commit_author_link} committed %{commit_timeago}" -msgstr "%{commit_timeago} 에 %{commit_author_link} 이(가) 커밋하였습니다. " +msgstr "%{commit_timeago} 에 %{commit_author_link} 님이 커밋하였습니다. " msgid "1 pipeline" msgid_plural "%d pipelines" @@ -791,7 +794,7 @@ msgid "Select target branch" msgstr "대상 브랜치 선택" msgid "Set a password on your account to pull or push via %{protocol}." -msgstr "%{protocol}을(를) 통해 Pull 하거나 Push하려면 계정에 패스워드를 설정하십시오." +msgstr "%{protocol} 프로토콜을 통해 Pull 하거나 Push하려면 계정에 패스워드를 설정하십시오." msgid "Set up CI" msgstr "CI 설정" @@ -1122,7 +1125,7 @@ msgid "" "You are going to remove %{group_name}.\n" "Removed groups CANNOT be restored!\n" "Are you ABSOLUTELY sure?" -msgstr "%{group_name}을(를) 제거하려고합니다.\n" +msgstr "%{group_name} 그룹을 제거하려고합니다.\n" "\"정말로\" 확실합니까?" msgid "" @@ -1130,7 +1133,7 @@ msgid "" "Removed project CANNOT be restored!\n" "Are you ABSOLUTELY sure?" msgstr "" -"%{project_name_with_namespace}을(를) 삭제하려고합니다.\n" +"%{project_name_with_namespace} 프로젝트를 삭제하려고합니다.\n" "삭제된 프로젝트를 복원 할 수 없습니다!\n" "\"정말로\" 확실합니까?" @@ -1185,7 +1188,7 @@ msgid "" "You won't be able to pull or push project code via SSH until you " "%{add_ssh_key_link} to your profile" msgstr "" -"당신의 프로필에 %{add_ssh_key_link} 을(를) 하기 전에는 SSH를 통해 프로젝트 코드를 Pull 하거나 Push 할 수 " +"당신의 프로필에 %{add_ssh_key_link} 를 하기 전에는 SSH를 통해 프로젝트 코드를 Pull 하거나 Push 할 수 " "없습니다" msgid "Your name" diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 29c449d6aa9..3d21b695af4 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -127,7 +127,7 @@ describe Admin::UsersController do describe 'POST create' do it 'creates the user' do - expect{ post :create, user: attributes_for(:user) }.to change{ User.count }.by(1) + expect { post :create, user: attributes_for(:user) }.to change { User.count }.by(1) end it 'shows only one error message for an invalid email' do diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index e478a253b3f..e00403118a0 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -24,7 +24,7 @@ describe InvitesController do describe 'GET #decline' do it 'declines user' do get :decline, id: token - expect{member.reload}.to raise_error ActiveRecord::RecordNotFound + expect {member.reload}.to raise_error ActiveRecord::RecordNotFound expect(response).to have_http_status(302) expect(flash[:notice]).to include 'You have declined the invitation to join' diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index bdee3894a13..23601c457b0 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -292,13 +292,13 @@ describe Projects::IssuesController do it 'rejects an issue recognized as a spam' do expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true) - expect { update_spam_issue }.not_to change{ issue.reload.title } + expect { update_spam_issue }.not_to change { issue.reload.title } end it 'rejects an issue recognized as a spam when recaptcha disabled' do stub_application_setting(recaptcha_enabled: false) - expect { update_spam_issue }.not_to change{ issue.reload.title } + expect { update_spam_issue }.not_to change { issue.reload.title } end it 'creates a spam log' do @@ -358,7 +358,7 @@ describe Projects::IssuesController do end it 'accepts an issue after recaptcha is verified' do - expect{ update_verified_issue }.to change{ issue.reload.title }.to(spammy_title) + expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) end it 'marks spam log as recaptcha_verified' do diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb index 974330e2bbd..41d211ed1bb 100644 --- a/spec/controllers/projects/todos_controller_spec.rb +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -67,7 +67,7 @@ describe Projects::TodosController do end it "doesn't create todo" do - expect{ go }.not_to change { user.todos.count } + expect { go }.not_to change { user.todos.count } expect(response).to have_http_status(404) end end @@ -135,7 +135,7 @@ describe Projects::TodosController do end it "doesn't create todo" do - expect{ go }.not_to change { user.todos.count } + expect { go }.not_to change { user.todos.count } expect(response).to have_http_status(404) end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 634563fc290..275181a3d64 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -15,7 +15,7 @@ describe RegistrationsController do it 'signs the user in' do allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(false) - expect { post(:create, user_params) }.not_to change{ ActionMailer::Base.deliveries.size } + expect { post(:create, user_params) }.not_to change { ActionMailer::Base.deliveries.size } expect(subject.current_user).not_to be_nil end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 1c494b8c7ab..225753333ee 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -138,7 +138,7 @@ describe Snippets::NotesController do end it "deletes the note" do - expect{ delete :destroy, request_params }.to change{ Note.count }.from(1).to(0) + expect { delete :destroy, request_params }.to change { Note.count }.from(1).to(0) end context 'system note' do @@ -147,7 +147,7 @@ describe Snippets::NotesController do end it "does not delete the note" do - expect{ delete :destroy, request_params }.not_to change{ Note.count } + expect { delete :destroy, request_params }.not_to change { Note.count } end end end @@ -166,7 +166,7 @@ describe Snippets::NotesController do end it "does not update the note" do - expect{ delete :destroy, request_params }.not_to change{ Note.count } + expect { delete :destroy, request_params }.not_to change { Note.count } end end end diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb index 5e1e7dc078f..b45972b7f6b 100644 --- a/spec/features/password_reset_spec.rb +++ b/spec/features/password_reset_spec.rb @@ -16,7 +16,7 @@ feature 'Password reset' do user.send_reset_password_instructions user.update_attribute(:reset_password_sent_at, 5.minutes.ago) - expect{ forgot_password(user) }.to change{ user.reset_password_sent_at } + expect { forgot_password(user) }.to change { user.reset_password_sent_at } expect(page).to have_content(I18n.t('devise.passwords.send_paranoid_instructions')) expect(current_path).to eq new_user_session_path end @@ -27,7 +27,7 @@ feature 'Password reset' do # Reload because PG handles datetime less precisely than Ruby/Rails user.reload - expect{ forgot_password(user) }.not_to change{ user.reset_password_sent_at } + expect { forgot_password(user) }.not_to change { user.reset_password_sent_at } expect(page).to have_content(I18n.t('devise.passwords.send_paranoid_instructions')) expect(current_path).to eq new_user_session_path end diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index 9123aa9d155..c935cdfd5c4 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Profile > Preferences' do +describe 'Profile > Preferences', :js do let(:user) { create(:user) } before do @@ -8,28 +8,32 @@ describe 'Profile > Preferences' do visit profile_preferences_path end - describe 'User changes their syntax highlighting theme', js: true do + describe 'User changes their syntax highlighting theme' do it 'creates a flash message' do choose 'user_color_scheme_id_5' + wait_for_requests + expect_preferences_saved_message end it 'updates their preference' do choose 'user_color_scheme_id_5' - allowing_for_delay do - visit page.current_path - expect(page).to have_checked_field('user_color_scheme_id_5') - end + wait_for_requests + refresh + + expect(page).to have_checked_field('user_color_scheme_id_5') end end - describe 'User changes their default dashboard', js: true do + describe 'User changes their default dashboard' do it 'creates a flash message' do select 'Starred Projects', from: 'user_dashboard' click_button 'Save' + wait_for_requests + expect_preferences_saved_message end @@ -37,12 +41,12 @@ describe 'Profile > Preferences' do select 'Starred Projects', from: 'user_dashboard' click_button 'Save' - allowing_for_delay do - find('#logo').click + wait_for_requests + + find('#logo').click - expect(page).to have_content("You don't have starred projects yet") - expect(page.current_path).to eq starred_dashboard_projects_path - end + expect(page).to have_content("You don't have starred projects yet") + expect(page.current_path).to eq starred_dashboard_projects_path find('.shortcuts-activity').trigger('click') diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index ff004d85272..1124b42721f 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -104,7 +104,7 @@ feature 'Users', js: true do end def errors_on_page(page) - page.find('#error_explanation').find('ul').all('li').map{ |item| item.text }.join("\n") + page.find('#error_explanation').find('ul').all('li').map { |item| item.text }.join("\n") end def number_of_errors_on_page(page) diff --git a/spec/javascripts/fixtures/blob.rb b/spec/javascripts/fixtures/blob.rb new file mode 100644 index 00000000000..16490ad5039 --- /dev/null +++ b/spec/javascripts/fixtures/blob.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Projects::BlobController, '(JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} + let(:project) { create(:project, :repository, namespace: namespace, path: 'branches-project') } + + render_views + + before(:all) do + clean_frontend_fixtures('blob/') + end + + before(:each) do + sign_in(admin) + end + + it 'blob/show.html.raw' do |example| + get(:show, + namespace_id: project.namespace, + project_id: project, + id: 'add-ipython-files/files/ipython/basic.ipynb') + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end +end diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index ab74f3e00ec..d7b7acaa3f4 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,9 +1,11 @@ /* global bp */ +import Cookies from 'js-cookie'; import { calculateTop, hideSubLevelItems, showSubLevelItems, canShowSubItems, + canShowActiveSubItems, } from '~/fly_out_nav'; describe('Fly out sidebar navigation', () => { @@ -57,11 +59,11 @@ describe('Fly out sidebar navigation', () => { expect( el.querySelector('.sidebar-sub-level-items').style.display, - ).toBe('none'); + ).toBe(''); }); it('does not hude subitems on mobile', () => { - breakpointSize = 'sm'; + breakpointSize = 'xs'; hideSubLevelItems(el); @@ -121,7 +123,7 @@ describe('Fly out sidebar navigation', () => { }); it('does not show sub-items on mobile', () => { - breakpointSize = 'sm'; + breakpointSize = 'xs'; showSubLevelItems(el); @@ -170,11 +172,59 @@ describe('Fly out sidebar navigation', () => { }); it('returns false if on mobile size', () => { - breakpointSize = 'sm'; + breakpointSize = 'xs'; expect( canShowSubItems(), ).toBeFalsy(); }); }); + + describe('canShowActiveSubItems', () => { + afterEach(() => { + Cookies.remove('sidebar_collapsed'); + }); + + it('returns true by default', () => { + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns false when cookie is false & element is active', () => { + Cookies.set('sidebar_collapsed', 'false'); + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeFalsy(); + }); + + it('returns true when cookie is false & element is active', () => { + Cookies.set('sidebar_collapsed', 'true'); + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns true when element is active & breakpoint is sm', () => { + breakpointSize = 'sm'; + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns true when element is active & breakpoint is md', () => { + breakpointSize = 'md'; + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + }); }); diff --git a/spec/lib/bitbucket/paginator_spec.rb b/spec/lib/bitbucket/paginator_spec.rb index 2c972da682e..bdf10a5e2a2 100644 --- a/spec/lib/bitbucket/paginator_spec.rb +++ b/spec/lib/bitbucket/paginator_spec.rb @@ -15,7 +15,7 @@ describe Bitbucket::Paginator do expect(paginator.items).to match(['item_2']) allow(paginator).to receive(:fetch_next_page).and_return(nil) - expect{ paginator.items }.to raise_error(StopIteration) + expect { paginator.items }.to raise_error(StopIteration) end end end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index ed571a2ba05..ee28387cd48 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -1323,11 +1323,11 @@ EOT describe "Error handling" do it "fails to parse YAML" do - expect{GitlabCiYamlProcessor.new("invalid: yaml: test")}.to raise_error(Psych::SyntaxError) + expect {GitlabCiYamlProcessor.new("invalid: yaml: test")}.to raise_error(Psych::SyntaxError) end it "indicates that object is invalid" do - expect{GitlabCiYamlProcessor.new("invalid_yaml")}.to raise_error(GitlabCiYamlProcessor::ValidationError) + expect {GitlabCiYamlProcessor.new("invalid_yaml")}.to raise_error(GitlabCiYamlProcessor::ValidationError) end it "returns errors if tags parameter is invalid" do diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index 867f7d55af7..e13406d1972 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -56,7 +56,7 @@ describe ExtractsPath do context 'subclass overrides get_id' do it 'uses ref returned by get_id' do - allow_any_instance_of(self.class).to receive(:get_id){ '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' } + allow_any_instance_of(self.class).to receive(:get_id) { '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' } assign_ref_vars diff --git a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb index d7f7b26740d..ae5b31dc12d 100644 --- a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::BitbucketImport::ProjectCreator do has_wiki?: false) end - let(:namespace){ create(:group, owner: user) } + let(:namespace) { create(:group, owner: user) } let(:token) { "asdasd12345" } let(:secret) { "sekrettt" } let(:access_params) { { bitbucket_access_token: token, bitbucket_access_token_secret: secret } } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 858616117d5..a90c848432e 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -422,11 +422,11 @@ describe Gitlab::Git::Repository, seed_helper: true do it "should fail if we create an existing branch" do @repo.create_branch('duplicated_branch', 'master') - expect{@repo.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") + expect {@repo.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") end it "should fail if we create a branch from a non existing ref" do - expect{@repo.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") + expect {@repo.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") end after(:all) do diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb deleted file mode 100644 index 5c9c4ed1d7c..00000000000 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'spec_helper' - -describe Gitlab::GitalyClient::RepositoryService do - set(:project) { create(:project) } - let(:storage_name) { project.repository_storage } - let(:relative_path) { project.path_with_namespace + '.git' } - let(:client) { described_class.new(project.repository) } - - describe '#exists?' do - it 'sends an exists message' do - expect_any_instance_of(Gitaly::RepositoryService::Stub) - .to receive(:exists) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) - .and_call_original - - client.exists? - end - end -end diff --git a/spec/lib/gitlab/gitlab_import/project_creator_spec.rb b/spec/lib/gitlab/gitlab_import/project_creator_spec.rb index da48d8f0670..82548c7fd31 100644 --- a/spec/lib/gitlab/gitlab_import/project_creator_spec.rb +++ b/spec/lib/gitlab/gitlab_import/project_creator_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::GitlabImport::ProjectCreator do owner: { name: "john" } }.with_indifferent_access end - let(:namespace){ create(:group, owner: user) } + let(:namespace) { create(:group, owner: user) } let(:token) { "asdffg" } let(:access_params) { { gitlab_access_token: token } } diff --git a/spec/lib/gitlab/google_code_import/project_creator_spec.rb b/spec/lib/gitlab/google_code_import/project_creator_spec.rb index aad53938d52..8d5b60d50de 100644 --- a/spec/lib/gitlab/google_code_import/project_creator_spec.rb +++ b/spec/lib/gitlab/google_code_import/project_creator_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::GoogleCodeImport::ProjectCreator do "repositoryUrls" => ["https://vim.googlecode.com/git/"] ) end - let(:namespace){ create(:group, owner: user) } + let(:namespace) { create(:group, owner: user) } before do namespace.add_owner(user) diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb index 574748756bd..cd5a1b2982b 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::ImportExport::AttributeCleaner do - let(:relation_class){ double('relation_class').as_null_object } + let(:relation_class) { double('relation_class').as_null_object } let(:unsafe_hash) do { 'id' => 101, diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb index 292ec064a67..ca2213cd112 100644 --- a/spec/lib/gitlab/ldap/config_spec.rb +++ b/spec/lib/gitlab/ldap/config_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::LDAP::Config do describe '#initialize' do it 'requires a provider' do - expect{ described_class.new }.to raise_error ArgumentError + expect { described_class.new }.to raise_error ArgumentError end it 'works' do @@ -15,7 +15,7 @@ describe Gitlab::LDAP::Config do end it 'raises an error if a unknown provider is used' do - expect{ described_class.new 'unknown' }.to raise_error(RuntimeError) + expect { described_class.new 'unknown' }.to raise_error(RuntimeError) end end diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 175ceec44d7..5100a5a609e 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -61,12 +61,12 @@ describe Gitlab::LDAP::User do it "finds the user if already existing" do create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') - expect{ ldap_user.save }.not_to change{ User.count } + expect { ldap_user.save }.not_to change { User.count } end it "connects to existing non-ldap user if the email matches" do existing_user = create(:omniauth_user, email: 'john@example.com', provider: "twitter") - expect{ ldap_user.save }.not_to change{ User.count } + expect { ldap_user.save }.not_to change { User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -75,7 +75,7 @@ describe Gitlab::LDAP::User do it 'connects to existing ldap user if the extern_uid changes' do existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain') - expect{ ldap_user.save }.not_to change{ User.count } + expect { ldap_user.save }.not_to change { User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -85,7 +85,7 @@ describe Gitlab::LDAP::User do it 'connects to existing ldap user if the extern_uid changes and email address has upper case characters' do existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain') - expect{ ldap_user_upper_case.save }.not_to change{ User.count } + expect { ldap_user_upper_case.save }.not_to change { User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -106,7 +106,7 @@ describe Gitlab::LDAP::User do end it "creates a new user if not found" do - expect{ ldap_user.save }.to change{ User.count }.by(1) + expect { ldap_user.save }.to change { User.count }.by(1) end context 'when signup is disabled' do diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 6c84c4a0c1e..15edb820908 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -147,7 +147,7 @@ describe Gitlab::OAuth::User do end it 'throws an error' do - expect{ oauth_user.save }.to raise_error StandardError + expect { oauth_user.save }.to raise_error StandardError end end @@ -157,7 +157,7 @@ describe Gitlab::OAuth::User do end it 'throws an error' do - expect{ oauth_user.save }.to raise_error StandardError + expect { oauth_user.save }.to raise_error StandardError end end end diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb index 2827a18515e..19710029224 100644 --- a/spec/lib/gitlab/saml/user_spec.rb +++ b/spec/lib/gitlab/saml/user_spec.rb @@ -109,7 +109,7 @@ describe Gitlab::Saml::User do end it 'does not throw an error' do - expect{ saml_user.save }.not_to raise_error + expect { saml_user.save }.not_to raise_error end end @@ -119,7 +119,7 @@ describe Gitlab::Saml::User do end it 'throws an error' do - expect{ saml_user.save }.to raise_error StandardError + expect { saml_user.save }.to raise_error StandardError end end end diff --git a/spec/lib/gitlab/sql/union_spec.rb b/spec/lib/gitlab/sql/union_spec.rb index 5346881444d..baf8f6644bf 100644 --- a/spec/lib/gitlab/sql/union_spec.rb +++ b/spec/lib/gitlab/sql/union_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::SQL::Union do empty_relation = User.none union = described_class.new([empty_relation, relation_1, relation_2]) - expect{User.where("users.id IN (#{union.to_sql})").to_a}.not_to raise_error + expect {User.where("users.id IN (#{union.to_sql})").to_a}.not_to raise_error expect(union.to_sql).to eq("#{to_sql(relation_1)}\nUNION\n#{to_sql(relation_2)}") end end diff --git a/spec/lib/gitlab/version_info_spec.rb b/spec/lib/gitlab/version_info_spec.rb index e7e1a92ae54..c8a1e433d59 100644 --- a/spec/lib/gitlab/version_info_spec.rb +++ b/spec/lib/gitlab/version_info_spec.rb @@ -50,8 +50,8 @@ describe 'Gitlab::VersionInfo' do context 'unknown' do it { expect(@unknown).not_to be @v0_0_1 } it { expect(@unknown).not_to be Gitlab::VersionInfo.new } - it { expect{@unknown > @v0_0_1}.to raise_error(ArgumentError) } - it { expect{@unknown < @v0_0_1}.to raise_error(ArgumentError) } + it { expect {@unknown > @v0_0_1}.to raise_error(ArgumentError) } + it { expect {@unknown < @v0_0_1}.to raise_error(ArgumentError) } end context 'parse' do diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb index e7022bd06f8..d6edc964844 100644 --- a/spec/lib/json_web_token/rsa_token_spec.rb +++ b/spec/lib/json_web_token/rsa_token_spec.rb @@ -27,7 +27,7 @@ describe JSONWebToken::RSAToken do subject { JWT.decode(rsa_encoded, rsa_key) } - it { expect{subject}.not_to raise_error } + it { expect {subject}.not_to raise_error } it { expect(subject.first).to include('key' => 'value') } it do expect(subject.second).to eq( @@ -41,7 +41,7 @@ describe JSONWebToken::RSAToken do let(:new_key) { OpenSSL::PKey::RSA.generate(512) } subject { JWT.decode(rsa_encoded, new_key) } - it { expect{subject}.to raise_error(JWT::DecodeError) } + it { expect {subject}.to raise_error(JWT::DecodeError) } end end end diff --git a/spec/lib/system_check/simple_executor_spec.rb b/spec/lib/system_check/simple_executor_spec.rb index 025ea2673b4..4de5da984ba 100644 --- a/spec/lib/system_check/simple_executor_spec.rb +++ b/spec/lib/system_check/simple_executor_spec.rb @@ -240,7 +240,7 @@ describe SystemCheck::SimpleExecutor do context 'when there is an exception' do it 'rescues the exception' do - expect{ subject.run_check(BugousCheck) }.not_to raise_exception + expect { subject.run_check(BugousCheck) }.not_to raise_exception end end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 6fb4794ea5f..8c4a366ef8f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -425,7 +425,7 @@ describe CommitStatus do end it "raise exception when trying to update" do - expect{ commit_status.save }.to raise_error(ActiveRecord::StaleObjectError) + expect { commit_status.save }.to raise_error(ActiveRecord::StaleObjectError) end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index c055863d298..6d825ba68d1 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -350,7 +350,7 @@ describe Issue do subject { create(:issue, project: create(:project, :repository)) } let(:backref_text) { "issue #{subject.to_reference}" } - let(:set_mentionable_text) { ->(txt){ subject.description = txt } } + let(:set_mentionable_text) { ->(txt) { subject.description = txt } } end it_behaves_like 'a Taskable' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a1a3e70a7d2..4aada17c8c0 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -681,7 +681,7 @@ describe MergeRequest do end it 'does not crash' do - expect{ subject.diverged_commits_count }.not_to raise_error + expect { subject.diverged_commits_count }.not_to raise_error end it 'returns 0' do @@ -753,7 +753,7 @@ describe MergeRequest do subject { create(:merge_request, :simple) } let(:backref_text) { "merge request #{subject.to_reference}" } - let(:set_mentionable_text) { ->(txt){ subject.description = txt } } + let(:set_mentionable_text) { ->(txt) { subject.description = txt } } end it_behaves_like 'a Taskable' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8f951605954..4a80f41cdc9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1301,7 +1301,7 @@ describe Project do subject { project.rename_repo } - it { expect{subject}.to raise_error(StandardError) } + it { expect {subject}.to raise_error(StandardError) } end end @@ -1832,6 +1832,11 @@ describe Project do describe '#change_head' do let(:project) { create(:project, :repository) } + it 'returns error if branch does not exist' do + expect(project.change_head('unexisted-branch')).to be false + expect(project.errors.size).to eq(1) + end + it 'calls the before_change_head and after_change_head methods' do expect(project.repository).to receive(:before_change_head) expect(project.repository).to receive(:after_change_head) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index cfa77648338..62f40c12c0a 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1224,7 +1224,7 @@ describe Repository, models: true do end describe 'skip_merges option' do - subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", limit: 100, skip_merges: true).map{ |k| k.id } } + subject { repository.commits(Gitlab::Git::BRANCH_REF_PREFIX + "'test'", limit: 100, skip_merges: true).map { |k| k.id } } it { is_expected.not_to include('e56497bb5f03a90a51293fc6d516788730953899') } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0103fb6040e..6c8248eeb40 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -118,6 +118,17 @@ describe User do expect(user).to validate_uniqueness_of(:username).case_insensitive end + + context 'when username is changed' do + let(:user) { build_stubbed(:user, username: 'old_path', namespace: build_stubbed(:namespace)) } + + it 'validates move_dir is allowed for the namespace' do + expect(user.namespace).to receive(:any_project_has_container_registry_tags?).and_return(true) + user.username = 'new_path' + expect(user).to be_invalid + expect(user.errors.messages[:username].first).to match('cannot be changed if a personal project has container registry tags') + end + end end it { is_expected.to validate_presence_of(:projects_limit) } diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 8b62aa268d9..3c02e6302b4 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -44,8 +44,8 @@ describe API::CommitStatuses do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(statuses_id).to contain_exactly(status3.id, status4.id, status5.id, status6.id) - json_response.sort_by!{ |status| status['id'] } - expect(json_response.map{ |status| status['allow_failure'] }).to eq([true, false, false, false]) + json_response.sort_by! { |status| status['id'] } + expect(json_response.map { |status| status['allow_failure'] }).to eq([true, false, false, false]) end end diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index d032d72de9c..e497ec333a2 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -182,7 +182,7 @@ describe API::DeployKeys do delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) expect(response).to have_http_status(204) - end.to change{ project.deploy_keys.count }.by(-1) + end.to change { project.deploy_keys.count }.by(-1) end it 'returns 404 Not Found with invalid ID' do diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 48db964d782..f1a26b6ce6c 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -140,33 +140,28 @@ describe API::Events do end context 'when exists some events' do + let(:merge_request1) { create(:merge_request, :closed, author: user, assignee: user, source_project: private_project, title: 'Test') } + let(:merge_request2) { create(:merge_request, :closed, author: user, assignee: user, source_project: private_project, title: 'Test') } + before do - create_event(note1) - create_event(note2) create_event(merge_request1) end - let(:note1) { create(:note_on_merge_request, project: private_project, author: user) } - let(:note2) { create(:note_on_issue, project: private_project, author: user) } - let(:merge_request1) { create(:merge_request, state: 'closed', author: user, assignee: user, source_project: private_project, title: 'Test') } - let(:merge_request2) { create(:merge_request, state: 'closed', author: user, assignee: user, source_project: private_project, title: 'Test') } - it 'avoids N+1 queries' do control_count = ActiveRecord::QueryRecorder.new do - get api("/projects/#{private_project.id}/events", user) + get api("/projects/#{private_project.id}/events", user), target_type: :merge_request end.count create_event(merge_request2) expect do - get api("/projects/#{private_project.id}/events", user) + get api("/projects/#{private_project.id}/events", user), target_type: :merge_request end.not_to exceed_query_limit(control_count) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response[0]).to include('target_type' => 'MergeRequest', 'target_id' => merge_request2.id) - expect(json_response[1]).to include('target_type' => 'MergeRequest', 'target_id' => merge_request1.id) + expect(json_response.size).to eq(2) + expect(json_response.map { |r| r['target_id'] }).to match_array([merge_request1.id, merge_request2.id]) end def create_event(target) diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 402ea057cc5..2179790d098 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -88,7 +88,7 @@ describe API::GroupVariables do it 'creates variable' do expect do post api("/groups/#{group.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true - end.to change{group.variables.count}.by(1) + end.to change {group.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -99,7 +99,7 @@ describe API::GroupVariables do it 'creates variable with optional attributes' do expect do post api("/groups/#{group.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2' - end.to change{group.variables.count}.by(1) + end.to change {group.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -110,7 +110,7 @@ describe API::GroupVariables do it 'does not allow to duplicate variable key' do expect do post api("/groups/#{group.id}/variables", user), key: variable.key, value: 'VALUE_2' - end.to change{group.variables.count}.by(0) + end.to change {group.variables.count}.by(0) expect(response).to have_http_status(400) end @@ -192,7 +192,7 @@ describe API::GroupVariables do delete api("/groups/#{group.id}/variables/#{variable.key}", user) expect(response).to have_http_status(204) - end.to change{group.variables.count}.by(-1) + end.to change {group.variables.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 7a1bd76af7a..d4006fe71a2 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -56,7 +56,7 @@ describe API::Helpers do end def doorkeeper_guard_returns(value) - allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value } + allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { value } end def error!(message, status, header) @@ -161,7 +161,7 @@ describe API::Helpers do describe "when authenticating using a user's private token" do it "returns nil for an invalid token" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' - allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false } + allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false } expect(current_user).to be_nil end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9eda6836ded..1e8eccb9b1c 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -326,7 +326,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -337,7 +337,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -348,7 +348,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['updated_at'] } + response_dates = json_response.map { |merge_request| merge_request['updated_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -359,7 +359,7 @@ describe API::MergeRequests do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 9ff2b782b52..1fc0ec528b9 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -53,7 +53,7 @@ describe API::PipelineSchedules do it 'returns matched pipeline schedules' do get api("/projects/#{project.id}/pipeline_schedules", developer), scope: target - expect(json_response.map{ |r| r['active'] }).to all(eq(active?(target))) + expect(json_response.map { |r| r['active'] }).to all(eq(active?(target))) end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index edd6516cf34..e9ee3dd679d 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -683,7 +683,7 @@ describe API::Runner do end context 'when job has been updated recently' do - it { expect{ patch_the_trace }.not_to change { job.updated_at }} + it { expect { patch_the_trace }.not_to change { job.updated_at }} it "changes the job's trace" do patch_the_trace @@ -692,7 +692,7 @@ describe API::Runner do end context 'when Runner makes a force-patch' do - it { expect{ force_patch_the_trace }.not_to change { job.updated_at }} + it { expect { force_patch_the_trace }.not_to change { job.updated_at }} it "doesn't change the build.trace" do force_patch_the_trace diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 3a95db030d4..c8ff25f70fa 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -36,7 +36,7 @@ describe API::Runners do it 'returns user available runners' do get api('/runners', user) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -46,7 +46,7 @@ describe API::Runners do it 'filters runners by scope' do get api('/runners?scope=active', user) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -74,7 +74,7 @@ describe API::Runners do it 'returns all runners' do get api('/runners/all', admin) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -93,7 +93,7 @@ describe API::Runners do it 'filters runners by scope' do get api('/runners/all?scope=specific', admin) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -277,7 +277,7 @@ describe API::Runners do delete api("/runners/#{shared_runner.id}", admin) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.shared.count }.by(-1) + end.to change { Ci::Runner.shared.count }.by(-1) end end @@ -287,7 +287,7 @@ describe API::Runners do delete api("/runners/#{unused_specific_runner.id}", admin) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end it 'deletes used runner' do @@ -295,7 +295,7 @@ describe API::Runners do delete api("/runners/#{specific_runner.id}", admin) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end @@ -330,7 +330,7 @@ describe API::Runners do delete api("/runners/#{specific_runner.id}", user) expect(response).to have_http_status(204) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end end @@ -349,7 +349,7 @@ describe API::Runners do it "returns project's runners" do get api("/projects/#{project.id}/runners", user) - shared = json_response.any?{ |r| r['is_shared'] } + shared = json_response.any? { |r| r['is_shared'] } expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array @@ -385,14 +385,14 @@ describe API::Runners do it 'enables specific runner' do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id - end.to change{ project.runners.count }.by(+1) + end.to change { project.runners.count }.by(+1) expect(response).to have_http_status(201) end it 'avoids changes when enabling already enabled runner' do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner.id - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(409) end @@ -401,7 +401,7 @@ describe API::Runners do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(403) end @@ -416,7 +416,7 @@ describe API::Runners do it 'enables any specific runner' do expect do post api("/projects/#{project.id}/runners", admin), runner_id: unused_specific_runner.id - end.to change{ project.runners.count }.by(+1) + end.to change { project.runners.count }.by(+1) expect(response).to have_http_status(201) end end @@ -461,7 +461,7 @@ describe API::Runners do delete api("/projects/#{project.id}/runners/#{two_projects_runner.id}", user) expect(response).to have_http_status(204) - end.to change{ project.runners.count }.by(-1) + end.to change { project.runners.count }.by(-1) end end @@ -469,7 +469,7 @@ describe API::Runners do it "does not disable project's runner" do expect do delete api("/projects/#{project.id}/runners/#{specific_runner.id}", user) - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(403) end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 373fab4d98a..d09b8bc42f1 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -52,10 +52,10 @@ describe API::Snippets do expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( public_snippet.id, public_snippet_other.id) - expect(json_response.map{ |snippet| snippet['web_url']} ).to include( + expect(json_response.map { |snippet| snippet['web_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}", "http://localhost/snippets/#{public_snippet_other.id}") - expect(json_response.map{ |snippet| snippet['raw_url']} ).to include( + expect(json_response.map { |snippet| snippet['raw_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}/raw", "http://localhost/snippets/#{public_snippet_other.id}/raw") end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index d5c53b703dd..572e9a0fd07 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -185,7 +185,7 @@ describe API::Triggers do expect do post api("/projects/#{project.id}/triggers", user), description: 'trigger' - end.to change{project.triggers.count}.by(1) + end.to change {project.triggers.count}.by(1) expect(response).to have_http_status(201) expect(json_response).to include('description' => 'trigger') @@ -288,7 +288,7 @@ describe API::Triggers do delete api("/projects/#{project.id}/triggers/#{trigger.id}", user) expect(response).to have_http_status(204) - end.to change{project.triggers.count}.by(-1) + end.to change {project.triggers.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing trigger' do diff --git a/spec/requests/api/v3/deploy_keys_spec.rb b/spec/requests/api/v3/deploy_keys_spec.rb index 13a62423b1d..2affd0cfa51 100644 --- a/spec/requests/api/v3/deploy_keys_spec.rb +++ b/spec/requests/api/v3/deploy_keys_spec.rb @@ -87,7 +87,7 @@ describe API::V3::DeployKeys do expect do post v3_api("/projects/#{project.id}/#{path}", admin), key_attrs - end.to change{ project.deploy_keys.count }.by(1) + end.to change { project.deploy_keys.count }.by(1) end it 'returns an existing ssh key when attempting to add a duplicate' do @@ -122,7 +122,7 @@ describe API::V3::DeployKeys do it 'should delete existing key' do expect do delete v3_api("/projects/#{project.id}/#{path}/#{deploy_key.id}", admin) - end.to change{ project.deploy_keys.count }.by(-1) + end.to change { project.deploy_keys.count }.by(-1) end it 'should return 404 Not Found with invalid ID' do diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index ef7516fc28f..18d0a804137 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -90,7 +90,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -99,7 +99,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -108,7 +108,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['updated_at'] } + response_dates = json_response.map { |merge_request| merge_request['updated_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -117,7 +117,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end end diff --git a/spec/requests/api/v3/project_snippets_spec.rb b/spec/requests/api/v3/project_snippets_spec.rb index 758fb482374..3963924a066 100644 --- a/spec/requests/api/v3/project_snippets_spec.rb +++ b/spec/requests/api/v3/project_snippets_spec.rb @@ -30,7 +30,7 @@ describe API::ProjectSnippets do expect(response).to have_http_status(200) expect(json_response.size).to eq(3) - expect(json_response.map{ |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id) + expect(json_response.map { |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id) expect(json_response.last).to have_key('web_url') end diff --git a/spec/requests/api/v3/runners_spec.rb b/spec/requests/api/v3/runners_spec.rb index 78660afd840..a31eb3f1d43 100644 --- a/spec/requests/api/v3/runners_spec.rb +++ b/spec/requests/api/v3/runners_spec.rb @@ -38,7 +38,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{shared_runner.id}", admin) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.shared.count }.by(-1) + end.to change { Ci::Runner.shared.count }.by(-1) end end @@ -48,7 +48,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{unused_specific_runner.id}", admin) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end it 'deletes used runner' do @@ -56,7 +56,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{specific_runner.id}", admin) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end @@ -91,7 +91,7 @@ describe API::V3::Runners do delete v3_api("/runners/#{specific_runner.id}", user) expect(response).to have_http_status(200) - end.to change{ Ci::Runner.specific.count }.by(-1) + end.to change { Ci::Runner.specific.count }.by(-1) end end end @@ -113,7 +113,7 @@ describe API::V3::Runners do delete v3_api("/projects/#{project.id}/runners/#{two_projects_runner.id}", user) expect(response).to have_http_status(200) - end.to change{ project.runners.count }.by(-1) + end.to change { project.runners.count }.by(-1) end end @@ -121,7 +121,7 @@ describe API::V3::Runners do it "does not disable project's runner" do expect do delete v3_api("/projects/#{project.id}/runners/#{specific_runner.id}", user) - end.to change{ project.runners.count }.by(0) + end.to change { project.runners.count }.by(0) expect(response).to have_http_status(403) end end diff --git a/spec/requests/api/v3/snippets_spec.rb b/spec/requests/api/v3/snippets_spec.rb index 1bc2258ebd3..9ead3cad8bb 100644 --- a/spec/requests/api/v3/snippets_spec.rb +++ b/spec/requests/api/v3/snippets_spec.rb @@ -45,10 +45,10 @@ describe API::V3::Snippets do expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( public_snippet.id, public_snippet_other.id) - expect(json_response.map{ |snippet| snippet['web_url']} ).to include( + expect(json_response.map { |snippet| snippet['web_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}", "http://localhost/snippets/#{public_snippet_other.id}") - expect(json_response.map{ |snippet| snippet['raw_url']} ).to include( + expect(json_response.map { |snippet| snippet['raw_url']} ).to include( "http://localhost/snippets/#{public_snippet.id}/raw", "http://localhost/snippets/#{public_snippet_other.id}/raw") end diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb index 60212660fb6..075de2c0cba 100644 --- a/spec/requests/api/v3/triggers_spec.rb +++ b/spec/requests/api/v3/triggers_spec.rb @@ -171,7 +171,7 @@ describe API::V3::Triggers do it 'creates trigger' do expect do post v3_api("/projects/#{project.id}/triggers", user) - end.to change{project.triggers.count}.by(1) + end.to change {project.triggers.count}.by(1) expect(response).to have_http_status(201) expect(json_response).to be_a(Hash) @@ -202,7 +202,7 @@ describe API::V3::Triggers do delete v3_api("/projects/#{project.id}/triggers/#{trigger.token}", user) expect(response).to have_http_status(200) - end.to change{project.triggers.count}.by(-1) + end.to change {project.triggers.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing trigger' do diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 098a0d8ca7d..48592e12822 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -74,7 +74,7 @@ describe API::Variables do it 'creates variable' do expect do post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true - end.to change{project.variables.count}.by(1) + end.to change {project.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -85,7 +85,7 @@ describe API::Variables do it 'creates variable with optional attributes' do expect do post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2' - end.to change{project.variables.count}.by(1) + end.to change {project.variables.count}.by(1) expect(response).to have_http_status(201) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -96,7 +96,7 @@ describe API::Variables do it 'does not allow to duplicate variable key' do expect do post api("/projects/#{project.id}/variables", user), key: variable.key, value: 'VALUE_2' - end.to change{project.variables.count}.by(0) + end.to change {project.variables.count}.by(0) expect(response).to have_http_status(400) end @@ -166,7 +166,7 @@ describe API::Variables do delete api("/projects/#{project.id}/variables/#{variable.key}", user) expect(response).to have_http_status(204) - end.to change{project.variables.count}.by(-1) + end.to change {project.variables.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index c077c458163..ebd67eb1e94 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -421,7 +421,7 @@ describe Ci::API::Builds do end context 'when build has been updated recently' do - it { expect{ patch_the_trace }.not_to change { build.updated_at }} + it { expect { patch_the_trace }.not_to change { build.updated_at }} it 'changes the build trace' do patch_the_trace @@ -430,7 +430,7 @@ describe Ci::API::Builds do end context 'when Runner makes a force-patch' do - it { expect{ force_patch_the_trace }.not_to change { build.updated_at }} + it { expect { force_patch_the_trace }.not_to change { build.updated_at }} it "doesn't change the build.trace" do force_patch_the_trace diff --git a/spec/serializers/analytics_build_entity_spec.rb b/spec/serializers/analytics_build_entity_spec.rb index 86e703a6448..9f26d5cd09a 100644 --- a/spec/serializers/analytics_build_entity_spec.rb +++ b/spec/serializers/analytics_build_entity_spec.rb @@ -47,7 +47,7 @@ describe AnalyticsBuildEntity do let(:finished_at) { nil } it 'does not blow up' do - expect{ subject[:date] }.not_to raise_error + expect { subject[:date] }.not_to raise_error end it 'shows the right message' do @@ -63,7 +63,7 @@ describe AnalyticsBuildEntity do let(:started_at) { nil } it 'does not blow up' do - expect{ subject[:date] }.not_to raise_error + expect { subject[:date] }.not_to raise_error end it 'shows the right message' do @@ -79,7 +79,7 @@ describe AnalyticsBuildEntity do let(:finished_at) { nil } it 'does not blow up' do - expect{ subject[:date] }.not_to raise_error + expect { subject[:date] }.not_to raise_error end it 'shows the right message' do diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 362d754bca3..2de8daba6b5 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -111,7 +111,7 @@ describe PipelineSerializer do shared_examples 'no N+1 queries' do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(59) + expect(recorded.count).to be_within(1).of(57) expect(recorded.cached_count).to eq(0) end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 82724ccd281..a6449a3c9f5 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -132,7 +132,7 @@ describe GitPushService, services: true do end it "creates a new pipeline" do - expect{ subject }.to change{ Ci::Pipeline.count } + expect { subject }.to change { Ci::Pipeline.count } expect(Ci::Pipeline.last).to be_push end end diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index f877c145390..05695aa8188 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -39,7 +39,7 @@ describe GitTagPushService do end it "creates a new pipeline" do - expect{ subject }.to change{ Ci::Pipeline.count } + expect { subject }.to change { Ci::Pipeline.count } expect(Ci::Pipeline.last).to be_push end end diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index befa65ec0f8..bdaab88d673 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -118,7 +118,7 @@ describe Issuable::BulkUpdateService do context 'when the new assignee ID is not present' do it 'does not unassign' do expect { bulk_update(issue, assignee_ids: []) } - .not_to change{ issue.reload.assignees } + .not_to change { issue.reload.assignees } end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index fcbd69fd58b..9b2d9e79f4f 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -351,14 +351,14 @@ describe Issues::CreateService do end it 'marks related spam_log as recaptcha_verified' do - expect { issue }.to change{SpamLog.last.recaptcha_verified}.from(false).to(true) + expect { issue }.to change {SpamLog.last.recaptcha_verified}.from(false).to(true) end context 'when spam log does not belong to a user' do let(:log_user) { create(:user) } it 'does not mark spam_log as recaptcha_verified' do - expect { issue }.not_to change{SpamLog.last.recaptcha_verified} + expect { issue }.not_to change {SpamLog.last.recaptcha_verified} end end end @@ -378,7 +378,7 @@ describe Issues::CreateService do end it 'creates a new spam_log' do - expect{issue}.to change{SpamLog.count}.from(0).to(1) + expect {issue}.to change {SpamLog.count}.from(0).to(1) end it 'assigns a spam_log to an issue' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 814e2cfbed0..34fb16edc84 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -478,7 +478,7 @@ describe Issues::UpdateService, :mailer do feature_visibility_attr = :"#{issue.model_name.plural}_access_level" project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) - expect{ update_issue(assignee_ids: [assignee.id]) }.not_to change{ issue.assignees } + expect { update_issue(assignee_ids: [assignee.id]) }.not_to change { issue.assignees } end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 9368594bc86..681feee61d1 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -509,7 +509,7 @@ describe MergeRequests::UpdateService, :mailer do feature_visibility_attr = :"#{merge_request.model_name.plural}_access_level" project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) - expect{ update_merge_request(assignee_id: assignee) }.not_to change{ merge_request.assignee } + expect { update_merge_request(assignee_id: assignee) }.not_to change { merge_request.assignee } end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 6af5c79135d..bd8ff5aaaa7 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -85,7 +85,7 @@ describe NotificationService, :mailer do it { expect(notification.new_key(key)).to be_truthy } it 'sends email to key owner' do - expect{ notification.new_key(key) }.to change{ ActionMailer::Base.deliveries.size }.by(1) + expect { notification.new_key(key) }.to change { ActionMailer::Base.deliveries.size }.by(1) end end end @@ -97,7 +97,7 @@ describe NotificationService, :mailer do it { expect(notification.new_gpg_key(key)).to be_truthy } it 'sends email to key owner' do - expect{ notification.new_gpg_key(key) }.to change{ ActionMailer::Base.deliveries.size }.by(1) + expect { notification.new_gpg_key(key) }.to change { ActionMailer::Base.deliveries.size }.by(1) end end end @@ -109,7 +109,7 @@ describe NotificationService, :mailer do it { expect(notification.new_email(email)).to be_truthy } it 'sends email to email owner' do - expect{ notification.new_email(email) }.to change{ ActionMailer::Base.deliveries.size }.by(1) + expect { notification.new_email(email) }.to change { ActionMailer::Base.deliveries.size }.by(1) end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d945e0aa1f0..1b282e82187 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -101,6 +101,13 @@ describe Projects::UpdateService, '#execute' do expect(Project.find(project.id).default_branch).to eq 'feature' end + + it 'does not change a default branch' do + # The branch 'unexisted-branch' does not exist. + update_project(project, admin, default_branch: 'unexisted-branch') + + expect(Project.find(project.id).default_branch).to eq 'master' + end end context 'when updating a project that contains container images' do diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb index 5698101af54..9fa5983db66 100644 --- a/spec/services/protected_branches/update_service_spec.rb +++ b/spec/services/protected_branches/update_service_spec.rb @@ -19,7 +19,7 @@ describe ProtectedBranches::UpdateService do let(:user) { create(:user) } it "raises error" do - expect{ service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) end end end diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb index d333430088d..2ece4e3b07b 100644 --- a/spec/services/protected_tags/update_service_spec.rb +++ b/spec/services/protected_tags/update_service_spec.rb @@ -19,7 +19,7 @@ describe ProtectedTags::UpdateService do let(:user) { create(:user) } it "raises error" do - expect{ service.execute(protected_tag) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect { service.execute(protected_tag) }.to raise_error(Gitlab::Access::AccessDeniedError) end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 8f1eb4863d9..6d36affa9dc 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -35,7 +35,7 @@ describe SystemNoteService do context 'metadata' do it 'creates a new system note metadata record' do - expect { subject }.to change{ SystemNoteMetadata.count }.from(0).to(1) + expect { subject }.to change { SystemNoteMetadata.count }.from(0).to(1) end it 'creates a record correctly' do @@ -477,7 +477,7 @@ describe SystemNoteService do end it 'does not create a system note metadata record' do - expect { subject }.not_to change{ SystemNoteMetadata.count } + expect { subject }.not_to change { SystemNoteMetadata.count } end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 80d05451e09..a9b34a5258a 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -586,13 +586,13 @@ describe TodoService do it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr_assigned, author: author) - expect{ service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count) + expect { service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count) end it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do create(:todo, :directly_addressed, user: skipped, project: project, target: addressed_mr_assigned, author: author) - expect{ service.update_merge_request(addressed_mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) + expect { service.update_merge_request(addressed_mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) end context 'with a task list' do diff --git a/spec/support/api/milestones_shared_examples.rb b/spec/support/api/milestones_shared_examples.rb index bf769225012..4bb5113957e 100644 --- a/spec/support/api/milestones_shared_examples.rb +++ b/spec/support/api/milestones_shared_examples.rb @@ -50,7 +50,7 @@ shared_examples_for 'group and project milestones' do |route_definition| expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(2) - expect(json_response.map{ |m| m['id'] }).to match_array([closed_milestone.id, other_milestone.id]) + expect(json_response.map { |m| m['id'] }).to match_array([closed_milestone.id, other_milestone.id]) end it 'does not return any milestone if none found' do diff --git a/spec/support/matchers/query_matcher.rb b/spec/support/matchers/query_matcher.rb index ac8c4ab91d9..bb0feca7c43 100644 --- a/spec/support/matchers/query_matcher.rb +++ b/spec/support/matchers/query_matcher.rb @@ -28,6 +28,6 @@ RSpec::Matchers.define :make_queries_matching do |matcher, expected_count = nil| def query_count(regex, &block) @recorder = ActiveRecord::QueryRecorder.new(&block).log - @recorder.select{ |q| q.match(regex) } + @recorder.select { |q| q.match(regex) } end end diff --git a/spec/support/rake_helpers.rb b/spec/support/rake_helpers.rb index 5cb415111d2..86bfeed107c 100644 --- a/spec/support/rake_helpers.rb +++ b/spec/support/rake_helpers.rb @@ -5,11 +5,15 @@ module RakeHelpers end def stub_warn_user_is_not_gitlab - allow_any_instance_of(Object).to receive(:warn_user_is_not_gitlab) + allow(main_object).to receive(:warn_user_is_not_gitlab) end def silence_output - allow($stdout).to receive(:puts) - allow($stdout).to receive(:print) + allow(main_object).to receive(:puts) + allow(main_object).to receive(:print) + end + + def main_object + @main_object ||= TOPLEVEL_BINDING.eval('self') end end diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb index 516f8878679..37c89d37aa0 100644 --- a/spec/support/stub_configuration.rb +++ b/spec/support/stub_configuration.rb @@ -38,6 +38,17 @@ module StubConfiguration allow(Gitlab.config.backup).to receive_messages(to_settings(messages)) end + def stub_storage_settings(messages) + messages.each do |storage_name, storage_settings| + storage_settings['failure_count_threshold'] ||= 10 + storage_settings['failure_wait_time'] ||= 30 + storage_settings['failure_reset_time'] ||= 1800 + storage_settings['storage_timeout'] ||= 5 + end + + allow(Gitlab.config.repositories).to receive(:storages).and_return(messages) + end + private # Modifies stubbed messages to also stub possible predicate versions diff --git a/spec/tasks/config_lint_spec.rb b/spec/tasks/config_lint_spec.rb index ed6c5b09663..5b01665019a 100644 --- a/spec/tasks/config_lint_spec.rb +++ b/spec/tasks/config_lint_spec.rb @@ -2,14 +2,14 @@ require 'rake_helper' Rake.application.rake_require 'tasks/config_lint' describe ConfigLint do - let(:files){ ['lib/support/fake.sh'] } + let(:files) { ['lib/support/fake.sh'] } it 'errors out if any bash scripts have errors' do - expect { described_class.run(files){ system('exit 1') } }.to raise_error(SystemExit) + expect { described_class.run(files) { system('exit 1') } }.to raise_error(SystemExit) end it 'passes if all scripts are fine' do - expect { described_class.run(files){ system('exit 0') } }.not_to raise_error + expect { described_class.run(files) { system('exit 0') } }.not_to raise_error end end diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index cc932a4ec4c..43ac1a72152 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -20,7 +20,7 @@ describe 'gitlab:gitaly namespace rake task' do context 'when an underlying Git command fail' do it 'aborts and display a help message' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).and_raise 'Git error' expect { run_rake_task('gitlab:gitaly:install', clone_path) }.to raise_error 'Git error' @@ -33,7 +33,7 @@ describe 'gitlab:gitaly namespace rake task' do end it 'calls checkout_or_clone_version with the right arguments' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).with(version: version, repo: repo, target_dir: clone_path) run_rake_task('gitlab:gitaly:install', clone_path) @@ -41,7 +41,7 @@ describe 'gitlab:gitaly namespace rake task' do end describe 'gmake/make' do - let(:command_preamble) { %w[/usr/bin/env -u RUBYOPT] } + let(:command_preamble) { %w[/usr/bin/env -u RUBYOPT -u BUNDLE_GEMFILE] } before(:all) do @old_env_ci = ENV.delete('CI') @@ -58,13 +58,13 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(main_object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) end it 'calls gmake in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0]) - expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end @@ -72,13 +72,13 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is not available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(main_object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) end it 'calls make in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42]) - expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb index ee3614c50f6..65155cb044d 100644 --- a/spec/tasks/gitlab/shell_rake_spec.rb +++ b/spec/tasks/gitlab/shell_rake_spec.rb @@ -22,7 +22,8 @@ describe 'gitlab:shell rake tasks' do describe 'create_hooks task' do it 'calls gitlab-shell bin/create_hooks' do expect_any_instance_of(Object).to receive(:system) - .with("#{Gitlab.config.gitlab_shell.path}/bin/create-hooks", *repository_storage_paths_args) + .with("#{Gitlab.config.gitlab_shell.path}/bin/create-hooks", + *Gitlab::TaskHelpers.repository_storage_paths_args) run_rake_task('gitlab:shell:create_hooks') end diff --git a/spec/tasks/gitlab/workhorse_rake_spec.rb b/spec/tasks/gitlab/workhorse_rake_spec.rb index 1b68f3044a4..42516d36c67 100644 --- a/spec/tasks/gitlab/workhorse_rake_spec.rb +++ b/spec/tasks/gitlab/workhorse_rake_spec.rb @@ -20,7 +20,7 @@ describe 'gitlab:workhorse namespace rake task' do context 'when an underlying Git command fail' do it 'aborts and display a help message' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).and_raise 'Git error' expect { run_rake_task('gitlab:workhorse:install', clone_path) }.to raise_error 'Git error' @@ -33,7 +33,7 @@ describe 'gitlab:workhorse namespace rake task' do end it 'calls checkout_or_clone_version with the right arguments' do - expect_any_instance_of(Object) + expect(main_object) .to receive(:checkout_or_clone_version).with(version: version, repo: repo, target_dir: clone_path) run_rake_task('gitlab:workhorse:install', clone_path) @@ -48,13 +48,13 @@ describe 'gitlab:workhorse namespace rake task' do context 'gmake is available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(Object).to receive(:run_command!).with(['gmake']).and_return(true) end it 'calls gmake in the gitlab-workhorse directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0]) - expect_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) + expect(main_object).to receive(:run_command!).with(['gmake']).and_return(true) run_rake_task('gitlab:workhorse:install', clone_path) end @@ -62,13 +62,13 @@ describe 'gitlab:workhorse namespace rake task' do context 'gmake is not available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) + expect(main_object).to receive(:checkout_or_clone_version) + allow(main_object).to receive(:run_command!).with(['make']).and_return(true) end it 'calls make in the gitlab-workhorse directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42]) - expect_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) + expect(main_object).to receive(:run_command!).with(['make']).and_return(true) run_rake_task('gitlab:workhorse:install', clone_path) end diff --git a/spec/workers/new_issue_worker_spec.rb b/spec/workers/new_issue_worker_spec.rb index ed49ce57c0b..4e15ccc534b 100644 --- a/spec/workers/new_issue_worker_spec.rb +++ b/spec/workers/new_issue_worker_spec.rb @@ -41,7 +41,7 @@ describe NewIssueWorker do let(:issue) { create(:issue, project: project, description: "issue for #{mentioned.to_reference}") } it 'creates a new event record' do - expect{ worker.perform(issue.id, user.id) }.to change { Event.count }.from(0).to(1) + expect { worker.perform(issue.id, user.id) }.to change { Event.count }.from(0).to(1) end it 'creates a notification for the assignee' do diff --git a/spec/workers/new_merge_request_worker_spec.rb b/spec/workers/new_merge_request_worker_spec.rb index 85af6184d39..9e0cbde45b1 100644 --- a/spec/workers/new_merge_request_worker_spec.rb +++ b/spec/workers/new_merge_request_worker_spec.rb @@ -43,7 +43,7 @@ describe NewMergeRequestWorker do end it 'creates a new event record' do - expect{ worker.perform(merge_request.id, user.id) }.to change { Event.count }.from(0).to(1) + expect { worker.perform(merge_request.id, user.id) }.to change { Event.count }.from(0).to(1) end it 'creates a notification for the assignee' do diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index 14ed8b7811e..75197039f5a 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -23,7 +23,7 @@ describe PipelineScheduleWorker do context 'when there is a scheduled pipeline within next_run_at' do it 'creates a new pipeline' do - expect{ subject }.to change { project.pipelines.count }.by(1) + expect { subject }.to change { project.pipelines.count }.by(1) expect(Ci::Pipeline.last).to be_schedule end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 74a9f90195c..af6a3c9f6c7 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -78,7 +78,7 @@ describe PostReceive do stub_ci_pipeline_to_return_yaml_file end - it { expect{ subject }.to change{ Ci::Pipeline.count }.by(2) } + it { expect { subject }.to change { Ci::Pipeline.count }.by(2) } end context "does not create a Ci::Pipeline" do @@ -86,7 +86,7 @@ describe PostReceive do stub_ci_pipeline_yaml_file(nil) end - it { expect{ subject }.not_to change{ Ci::Pipeline.count } } + it { expect { subject }.not_to change { Ci::Pipeline.count } } end end |