From bbc2ca40c2e1df0793c838d14ea3225a016fccbb Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Fri, 29 Jun 2018 18:04:03 -0500 Subject: [PATCH 01/38] save current date before Pikaday init --- app/assets/javascripts/due_date_select.js | 4 +++- ...ing-milestone-date-change-when-editing.yml | 5 +++++ .../milestones/user_edits_milestone_spec.rb | 22 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/41671-fixing-milestone-date-change-when-editing.yml create mode 100644 spec/features/milestones/user_edits_milestone_spec.rb diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index 4164149dd06..39bb838f683 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -170,6 +170,8 @@ export default class DueDateSelectors { initMilestoneDatePicker() { $('.datepicker').each(function initPikadayMilestone() { const $datePicker = $(this); + const datePickerVal = $datePicker.val(); + const calendar = new Pikaday({ field: $datePicker.get(0), theme: 'gitlab-theme animate-picker', @@ -182,7 +184,7 @@ export default class DueDateSelectors { }, }); - calendar.setDate(parsePikadayDate($datePicker.val())); + calendar.setDate(parsePikadayDate(datePickerVal)); $datePicker.data('pikaday', calendar); }); diff --git a/changelogs/unreleased/41671-fixing-milestone-date-change-when-editing.yml b/changelogs/unreleased/41671-fixing-milestone-date-change-when-editing.yml new file mode 100644 index 00000000000..c6a0dc4f129 --- /dev/null +++ b/changelogs/unreleased/41671-fixing-milestone-date-change-when-editing.yml @@ -0,0 +1,5 @@ +--- +title: "Fixing milestone date change when editing" +merge_request: 20279 +author: Orlando Del Aguila +type: fixed \ No newline at end of file diff --git a/spec/features/milestones/user_edits_milestone_spec.rb b/spec/features/milestones/user_edits_milestone_spec.rb new file mode 100644 index 00000000000..077295f1cc0 --- /dev/null +++ b/spec/features/milestones/user_edits_milestone_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" + +describe "User edits milestone", :js do + set(:user) { create(:user) } + set(:project) { create(:project) } + set(:milestone) { create(:milestone, project: project, start_date: Date.today, due_date: 5.days.from_now) } + + before do + project.add_developer(user) + sign_in(user) + + visit(edit_project_milestone_path(project, milestone)) + end + + it "shows the right start date and due date" do + start_date = milestone.start_date.strftime("%F") + due_date = milestone.due_date.strftime("%F") + + expect(page).to have_field(with: start_date) + expect(page).to have_field(with: due_date) + end +end From 9e85e37bdebc956636c7f67f0162e4174aac79e0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 6 Jul 2018 16:55:55 +0000 Subject: [PATCH 02/38] Remove badges from README.md in favor of GitLab badges feature --- README.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/README.md b/README.md index 77f03b791f2..b6e1cc9a432 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,5 @@ # GitLab -[![Build status](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/build.svg)](https://gitlab.com/gitlab-org/gitlab-ce/commits/master) -[![Overall test coverage](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg)](https://gitlab.com/gitlab-org/gitlab-ce/pipelines) -[![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq) -[![Core Infrastructure Initiative Best Practices](https://bestpractices.coreinfrastructure.org/projects/42/badge)](https://bestpractices.coreinfrastructure.org/projects/42) -[![Gitter](https://badges.gitter.im/gitlabhq/gitlabhq.svg)](https://gitter.im/gitlabhq/gitlabhq?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge) - ## Test coverage - [![Ruby coverage](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage)](https://gitlab-org.gitlab.io/gitlab-ce/coverage-ruby) Ruby From 11027efdfa02c414331bbd59e86339cf7e875e49 Mon Sep 17 00:00:00 2001 From: DJ Mountney Date: Fri, 6 Jul 2018 12:05:33 -0700 Subject: [PATCH 03/38] Use the correct script location for the build trigger script --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 49c4b059dbc..610a5ecba6d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -327,7 +327,7 @@ cloud-native-image: cache: {} script: - gem install gitlab --no-ri --no-rdoc - - ./trigger-build cng + - ./scripts/trigger-build cng only: - tags@gitlab-org/gitlab-ce - tags@gitlab-org/gitlab-ee From 7670c29f08291c308f18a4cf2fb1c0e0446bd613 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Fri, 6 Jul 2018 16:45:50 -0500 Subject: [PATCH 04/38] specify that wiki pags are rendered with Redcarpet --- doc/user/markdown.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 8e87c896a72..bd199b55a61 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -9,7 +9,7 @@ > For the best result, we encourage you to check this document out as rendered by GitLab: [markdown.md] -_GitLab uses (as of 11.1) the [CommonMark Ruby Library][commonmarker] for Markdown processing of all new issues, merge requests, comments, and other Markdown content in the GitLab system. Previous content and Markdown files `.md` in the repositories are still processed using the [Redcarpet Ruby library][redcarpet]._ +_GitLab uses (as of 11.1) the [CommonMark Ruby Library][commonmarker] for Markdown processing of all new issues, merge requests, comments, and other Markdown content in the GitLab system. Previous content, wiki pages and Markdown files (`.md`) in the repositories are still processed using the [Redcarpet Ruby library][redcarpet]._ _Where there are significant differences, we will try to call them out in this document._ @@ -22,7 +22,7 @@ You can use GFM in the following areas: - merge requests - milestones - snippets (the snippet must be named with a `.md` extension) -- wiki pages +- wiki pages (currently only rendered by Redcarpet) - markdown documents inside the repository (currently only rendered by Redcarpet) You can also use other rich text files in GitLab. You might have to install a From fbb48bd7e9e20037bb5a1b79e70f68f2d4bc3170 Mon Sep 17 00:00:00 2001 From: Jasper Maes Date: Sat, 7 Jul 2018 13:46:23 +0200 Subject: [PATCH 05/38] Rails5 fix mysql milliseconds problem in specs --- changelogs/unreleased/rails5-fix-48977.yml | 5 +++++ spec/models/user_spec.rb | 4 +++- spec/services/ci/retry_build_service_spec.rb | 12 ++++++++++-- 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/rails5-fix-48977.yml diff --git a/changelogs/unreleased/rails5-fix-48977.yml b/changelogs/unreleased/rails5-fix-48977.yml new file mode 100644 index 00000000000..bfd86f20e24 --- /dev/null +++ b/changelogs/unreleased/rails5-fix-48977.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 fix mysql milliseconds problem in specs +merge_request: 20464 +author: Jasper Maes +type: fixed diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 097144d04ce..57c408498dd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2461,7 +2461,9 @@ describe User do it 'changes the namespace (just to compare to when username is not changed)' do expect do - user.update_attributes!(username: new_username) + Timecop.freeze(1.second.from_now) do + user.update_attributes!(username: new_username) + end end.to change { user.namespace.updated_at } end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index decb5d22f59..b8cdbc8c0f6 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -100,7 +100,11 @@ describe Ci::RetryBuildService do end describe '#execute' do - let(:new_build) { service.execute(build) } + let(:new_build) do + Timecop.freeze(1.second.from_now) do + service.execute(build) + end + end context 'when user has ability to execute build' do before do @@ -150,7 +154,11 @@ describe Ci::RetryBuildService do end describe '#reprocess' do - let(:new_build) { service.reprocess!(build) } + let(:new_build) do + Timecop.freeze(1.second.from_now) do + service.reprocess!(build) + end + end context 'when user has ability to execute build' do before do From 3d88f870b7c427a1284935fad467c6c125d4479a Mon Sep 17 00:00:00 2001 From: Jasper Maes Date: Sat, 7 Jul 2018 14:08:07 +0200 Subject: [PATCH 06/38] Update Gemfile.rails5.lock with latest Gemfile.lock changes --- Gemfile.rails5.lock | 10 +++++----- changelogs/unreleased/rails5-update-gemfile-lock.yml | 5 +++++ 2 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/rails5-update-gemfile-lock.yml diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 8c46b8c5916..0bb2e67ab6d 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -79,7 +79,7 @@ GEM babosa (1.0.2) base32 (0.3.2) batch-loader (1.2.1) - bcrypt (3.1.11) + bcrypt (3.1.12) bcrypt_pbkdf (1.0.0) benchmark-ips (2.3.0) better_errors (2.1.1) @@ -111,7 +111,7 @@ GEM capybara-screenshot (1.0.14) capybara (>= 1.0, < 3) launchy - carrierwave (1.2.1) + carrierwave (1.2.3) activemodel (>= 4.0.0) activesupport (>= 4.0.0) mime-types (>= 1.16) @@ -514,7 +514,7 @@ GEM net-ssh (5.0.1) netrc (0.11.0) nio4r (2.3.1) - nokogiri (1.8.2) + nokogiri (1.8.3) mini_portile2 (~> 2.3.0) nokogumbo (1.5.0) nokogiri @@ -811,7 +811,7 @@ GEM rubyzip (1.2.1) rufus-scheduler (3.4.0) et-orbi (~> 1.0) - rugged (0.27.1) + rugged (0.27.2) safe_yaml (1.0.4) sanitize (4.6.5) crass (~> 1.0.2) @@ -877,7 +877,7 @@ GEM activesupport (>= 4.2) spring-commands-rspec (1.0.4) spring (>= 0.9.1) - sprockets (3.7.1) + sprockets (3.7.2) concurrent-ruby (~> 1.0) rack (> 1, < 3) sprockets-rails (3.2.1) diff --git a/changelogs/unreleased/rails5-update-gemfile-lock.yml b/changelogs/unreleased/rails5-update-gemfile-lock.yml new file mode 100644 index 00000000000..58931587fff --- /dev/null +++ b/changelogs/unreleased/rails5-update-gemfile-lock.yml @@ -0,0 +1,5 @@ +--- +title: Update Gemfile.rails5.lock with latest Gemfile.lock changes +merge_request: 20466 +author: Jasper Maes +type: fixed From e2ec97a92e6393dd0adeed39c77ff2b4eba0aed9 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Sat, 7 Jul 2018 19:30:16 +0200 Subject: [PATCH 07/38] Add FileUploader.root to allowed upload paths Currently we check if uploaded file is under `Gitlab.config.uploads.storage_path`, the problem is that uploads are placed in `uploads` subdirectory which is symlink. In allow_path? method we check real (expanded) paths, which causes that `Gitlab.config.uploads.storage_path` is expaned into symlink path and there is a mismatch with upload file path. By adding `Gitlab.config.uploads.storage_path/uploads` into allowed paths, this path is expaned during path check. `Gitlab.config.uploads.storage_path` is left there intentionally in case some uploader wouldn't use `uploads` subdir. --- changelogs/unreleased/jprovazn-upload-symlink.yml | 5 +++++ lib/gitlab/middleware/multipart.rb | 2 +- lib/uploaded_file.rb | 5 +++-- 3 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/jprovazn-upload-symlink.yml diff --git a/changelogs/unreleased/jprovazn-upload-symlink.yml b/changelogs/unreleased/jprovazn-upload-symlink.yml new file mode 100644 index 00000000000..265791d332f --- /dev/null +++ b/changelogs/unreleased/jprovazn-upload-symlink.yml @@ -0,0 +1,5 @@ +--- +title: Add /uploads subdirectory to allowed upload paths. +merge_request: +author: +type: fixed diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb index 9753be6d5c3..18f91db98fc 100644 --- a/lib/gitlab/middleware/multipart.rb +++ b/lib/gitlab/middleware/multipart.rb @@ -84,7 +84,7 @@ module Gitlab def open_file(params, key) ::UploadedFile.from_params( params, key, - Gitlab.config.uploads.storage_path) + [FileUploader.root, Gitlab.config.uploads.storage_path]) end end diff --git a/lib/uploaded_file.rb b/lib/uploaded_file.rb index 5dc85b2baea..0172461670b 100644 --- a/lib/uploaded_file.rb +++ b/lib/uploaded_file.rb @@ -28,7 +28,7 @@ class UploadedFile @tempfile = File.new(path, 'rb') end - def self.from_params(params, field, upload_path) + def self.from_params(params, field, upload_paths) unless params["#{field}.path"] raise InvalidPathError, "file is invalid" if params["#{field}.remote_id"] @@ -37,7 +37,8 @@ class UploadedFile file_path = File.realpath(params["#{field}.path"]) - unless self.allowed_path?(file_path, [upload_path, Dir.tmpdir].compact) + paths = Array.wrap(upload_paths) << Dir.tmpdir + unless self.allowed_path?(file_path, paths.compact) raise InvalidPathError, "insecure path used '#{file_path}'" end From d17e131f68939d3c74891fb381fbbf9e908c23ab Mon Sep 17 00:00:00 2001 From: Jasper Maes Date: Sun, 8 Jul 2018 12:42:37 +0200 Subject: [PATCH 08/38] Rails5 mysql fix milliseconds problem in pull request importer spec --- changelogs/unreleased/rails5-mysql-fix-pr-importer-spec.yml | 5 +++++ .../github_import/importer/pull_requests_importer_spec.rb | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/rails5-mysql-fix-pr-importer-spec.yml diff --git a/changelogs/unreleased/rails5-mysql-fix-pr-importer-spec.yml b/changelogs/unreleased/rails5-mysql-fix-pr-importer-spec.yml new file mode 100644 index 00000000000..afd9865ee45 --- /dev/null +++ b/changelogs/unreleased/rails5-mysql-fix-pr-importer-spec.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 mysql fix milliseconds problem in pull request importer spec +merge_request: 20475 +author: Jasper Maes +type: fixed diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb index 51fad6c6838..b2e544e6fed 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb @@ -27,9 +27,9 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do milestone: double(:milestone, number: 4), user: double(:user, id: 4, login: 'alice'), assignee: double(:user, id: 4, login: 'alice'), - created_at: Time.zone.now, - updated_at: Time.zone.now, - merged_at: Time.zone.now + created_at: 1.second.ago, + updated_at: 1.second.ago, + merged_at: 1.second.ago ) end From 4a3dae2717615c68f0961a201f9c9d2c0b5c3c61 Mon Sep 17 00:00:00 2001 From: Matthew Dawson Date: Mon, 2 Jul 2018 12:19:57 -0400 Subject: [PATCH 09/38] Update hamlit to fix ruby 2.5 incompatibilities, fixes #42045 Also update the hook logs to escape the contents properly, as the :plain filter does not do html escaping. --- Gemfile | 2 +- Gemfile.lock | 10 +++++----- Gemfile.rails5.lock | 8 ++++---- app/views/shared/hook_logs/_content.html.haml | 4 ++-- changelogs/unreleased/upgrade-hamlit-for-ruby25.yml | 5 +++++ 5 files changed, 17 insertions(+), 12 deletions(-) create mode 100644 changelogs/unreleased/upgrade-hamlit-for-ruby25.yml diff --git a/Gemfile b/Gemfile index 1fbc240bf6f..00ff75ae719 100644 --- a/Gemfile +++ b/Gemfile @@ -104,7 +104,7 @@ gem 'hashie-forbidden_attributes' gem 'kaminari', '~> 1.0' # HAML -gem 'hamlit', '~> 2.6.1' +gem 'hamlit', '~> 2.8.8' # Files attachments gem 'carrierwave', '~> 1.2' diff --git a/Gemfile.lock b/Gemfile.lock index a889c4dc3a0..ee5da96665c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -381,8 +381,8 @@ GEM rake (>= 10, < 13) rubocop (>= 0.49.0) sysexits (~> 1.1) - hamlit (2.6.1) - temple (~> 0.7.6) + hamlit (2.8.8) + temple (>= 0.8.0) thor tilt hashdiff (0.3.4) @@ -889,7 +889,7 @@ GEM sys-filesystem (1.1.6) ffi sysexits (1.2.0) - temple (0.7.7) + temple (0.8.0) test-prof (0.2.5) test_after_commit (1.1.0) activerecord (>= 3.2) @@ -900,7 +900,7 @@ GEM rack (>= 1, < 3) thor (0.19.4) thread_safe (0.3.6) - tilt (2.0.6) + tilt (2.0.8) timecop (0.8.1) timfel-krb5-auth (0.8.3) toml (0.1.2) @@ -1057,7 +1057,7 @@ DEPENDENCIES graphql (~> 1.8.0) grpc (~> 1.11.0) haml_lint (~> 0.26.0) - hamlit (~> 2.6.1) + hamlit (~> 2.8.8) hashie-forbidden_attributes health_check (~> 2.6.0) hipchat (~> 1.5.0) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 8c46b8c5916..66325234e29 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -384,8 +384,8 @@ GEM rake (>= 10, < 13) rubocop (>= 0.49.0) sysexits (~> 1.1) - hamlit (2.6.1) - temple (~> 0.7.6) + hamlit (2.8.8) + temple (>= 0.8.0) thor tilt hashdiff (0.3.4) @@ -898,7 +898,7 @@ GEM sys-filesystem (1.1.6) ffi sysexits (1.2.0) - temple (0.7.7) + temple (0.8.0) test-prof (0.2.5) text (1.3.1) thin (1.7.0) @@ -1067,7 +1067,7 @@ DEPENDENCIES graphql (~> 1.8.0) grpc (~> 1.11.0) haml_lint (~> 0.26.0) - hamlit (~> 2.6.1) + hamlit (~> 2.8.8) hashie-forbidden_attributes health_check (~> 2.6.0) hipchat (~> 1.5.0) diff --git a/app/views/shared/hook_logs/_content.html.haml b/app/views/shared/hook_logs/_content.html.haml index 532712ee6d1..f3b56df0c96 100644 --- a/app/views/shared/hook_logs/_content.html.haml +++ b/app/views/shared/hook_logs/_content.html.haml @@ -30,7 +30,7 @@ %h5 Request body: %pre - :plain + :escaped #{JSON.pretty_generate(hook_log.request_data)} %h5 Response headers: %pre @@ -40,5 +40,5 @@ %h5 Response body: %pre - :plain + :escaped #{hook_log.response_body} diff --git a/changelogs/unreleased/upgrade-hamlit-for-ruby25.yml b/changelogs/unreleased/upgrade-hamlit-for-ruby25.yml new file mode 100644 index 00000000000..39e10121507 --- /dev/null +++ b/changelogs/unreleased/upgrade-hamlit-for-ruby25.yml @@ -0,0 +1,5 @@ +--- +title: 'Update hamlit to fix ruby 2.5 incompatibilities, fixes #42045' +merge_request: +author: Matthew Dawson +type: fixed From 4a4338fcc60dcee33b0075213213ae51e8395a6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 27 Jun 2018 17:55:40 -0400 Subject: [PATCH 10/38] Use Gitaly's OperationService.UserUpdateBranch RPC --- lib/gitlab/git/repository.rb | 8 +++- lib/gitlab/gitaly_client/operation_service.rb | 16 ++++++++ .../gitaly_client/operation_service_spec.rb | 41 +++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 29b3663a52a..6fb1af4d057 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -651,7 +651,13 @@ module Gitlab end def update_branch(branch_name, user:, newrev:, oldrev:) - OperationService.new(user, self).update_branch(branch_name, newrev, oldrev) + gitaly_migrate(:operation_user_update_branch) do |is_enabled| + if is_enabled + gitaly_operations_client.user_update_branch(branch_name, user, newrev, oldrev) + else + OperationService.new(user, self).update_branch(branch_name, newrev, oldrev) + end + end end def rm_branch(branch_name, user:) diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index ab2c61f6782..555733d1834 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -68,6 +68,22 @@ module Gitlab raise Gitlab::Git::Repository::InvalidRef, ex end + def user_update_branch(branch_name, user, newrev, oldrev) + request = Gitaly::UserUpdateBranchRequest.new( + repository: @gitaly_repo, + branch_name: encode_binary(branch_name), + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, + newrev: encode_binary(newrev), + oldrev: encode_binary(oldrev) + ) + + response = GitalyClient.call(@repository.storage, :operation_service, :user_update_branch, request) + + if pre_receive_error = response.pre_receive_error.presence + raise Gitlab::Git::PreReceiveError, pre_receive_error + end + end + def user_delete_branch(branch_name, user) request = Gitaly::UserDeleteBranchRequest.new( repository: @gitaly_repo, diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 9709f1f5646..031d1e87dc1 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -53,6 +53,47 @@ describe Gitlab::GitalyClient::OperationService do end end + describe '#user_update_branch' do + let(:branch_name) { 'my-branch' } + let(:newrev) { '01e' } + let(:oldrev) { '01d' } + let(:request) do + Gitaly::UserUpdateBranchRequest.new( + repository: repository.gitaly_repository, + branch_name: branch_name, + newrev: newrev, + oldrev: oldrev, + user: gitaly_user + ) + end + let(:response) { Gitaly::UserUpdateBranchResponse.new } + + subject { client.user_update_branch(branch_name, user, newrev, oldrev) } + + it 'sends a user_update_branch message' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_update_branch).with(request, kind_of(Hash)) + .and_return(response) + + subject + end + + context "when pre_receive_error is present" do + let(:response) do + Gitaly::UserUpdateBranchResponse.new(pre_receive_error: "something failed") + end + + it "throws a PreReceive exception" do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_update_branch).with(request, kind_of(Hash)) + .and_return(response) + + expect { subject }.to raise_error( + Gitlab::Git::PreReceiveError, "something failed") + end + end + end + describe '#user_delete_branch' do let(:branch_name) { 'my-branch' } let(:request) do From a4e75e7a83082c190474595ed9894ec86061d37f Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 9 Jul 2018 12:50:17 +0200 Subject: [PATCH 11/38] Use Gitaly for fetches and creating bundles --- lib/gitlab/git/repository.rb | 16 +--- lib/gitlab/shell.rb | 44 +--------- spec/lib/gitlab/git/repository_spec.rb | 70 +++++++--------- spec/lib/gitlab/shell_spec.rb | 111 ++++++------------------- spec/lib/gitlab/workhorse_spec.rb | 24 +++--- 5 files changed, 76 insertions(+), 189 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 420790f45d0..00c136cab41 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -898,12 +898,8 @@ module Gitlab end def fetch_source_branch!(source_repository, source_branch, local_ref) - Gitlab::GitalyClient.migrate(:fetch_source_branch) do |is_enabled| - if is_enabled - gitaly_repository_client.fetch_source_branch(source_repository, source_branch, local_ref) - else - rugged_fetch_source_branch(source_repository, source_branch, local_ref) - end + wrapped_gitaly_errors do + gitaly_repository_client.fetch_source_branch(source_repository, source_branch, local_ref) end end @@ -1058,12 +1054,8 @@ module Gitlab end def bundle_to_disk(save_path) - gitaly_migrate(:bundle_to_disk) do |is_enabled| - if is_enabled - gitaly_repository_client.create_bundle(save_path) - else - run_git!(%W(bundle create #{save_path} --all)) - end + wrapped_gitaly_errors do + gitaly_repository_client.create_bundle(save_path) end true diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index e222541992a..a17cd27e82d 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -92,21 +92,13 @@ module Gitlab # Ex. # import_repository("nfs-file06", "gitlab/gitlab-ci", "https://gitlab.com/gitlab-org/gitlab-test.git") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/874 def import_repository(storage, name, url) if url.start_with?('.', '/') raise Error.new("don't use disk paths with import_repository: #{url.inspect}") end relative_path = "#{name}.git" - cmd = gitaly_migrate(:import_repository, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - GitalyGitlabProjects.new(storage, relative_path) - else - # The timeout ensures the subprocess won't hang forever - gitlab_projects(storage, relative_path) - end - end + cmd = GitalyGitlabProjects.new(storage, relative_path) success = cmd.import_project(url, git_timeout) raise Error, cmd.output unless success @@ -126,12 +118,8 @@ module Gitlab # fetch_remote(my_repo, "upstream") # def fetch_remote(repository, remote, ssh_auth: nil, forced: false, no_tags: false, prune: true) - gitaly_migrate(:fetch_remote) do |is_enabled| - if is_enabled - repository.gitaly_repository_client.fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, timeout: git_timeout, prune: prune) - else - local_fetch_remote(repository.storage, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, prune: prune) - end + wrapped_gitaly_errors do + repository.gitaly_repository_client.fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, timeout: git_timeout, prune: prune) end end @@ -389,28 +377,6 @@ module Gitlab ) end - def local_fetch_remote(storage_name, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false, prune: true) - vars = { force: forced, tags: !no_tags, prune: prune } - - if ssh_auth&.ssh_import? - if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? - vars[:ssh_key] = ssh_auth.ssh_private_key - end - - if ssh_auth.ssh_known_hosts.present? - vars[:known_hosts] = ssh_auth.ssh_known_hosts - end - end - - cmd = gitlab_projects(storage_name, repository_relative_path) - - success = cmd.fetch_remote(remote, git_timeout, vars) - - raise Error, cmd.output unless success - - success - end - def gitlab_shell_fast_execute(cmd) output, status = gitlab_shell_fast_execute_helper(cmd) @@ -440,10 +406,6 @@ module Gitlab Gitlab.config.gitlab_shell.git_timeout end - def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block) - wrapped_gitaly_errors { Gitlab::GitalyClient.migrate(method, status: status, &block) } - end - def wrapped_gitaly_errors yield rescue GRPC::NotFound, GRPC::BadStatus => e diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index e6268a05d44..4f8e6f29255 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1716,59 +1716,51 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#fetch_source_branch!' do - shared_examples '#fetch_source_branch!' do - let(:local_ref) { 'refs/merge-requests/1/head' } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - let(:source_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:local_ref) { 'refs/merge-requests/1/head' } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:source_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - after do - ensure_seeds - end + after do + ensure_seeds + end - context 'when the branch exists' do - context 'when the commit does not exist locally' do - let(:source_branch) { 'new-branch-for-fetch-source-branch' } - let(:source_rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { source_repository.rugged } } - let(:new_oid) { new_commit_edit_old_file(source_rugged).oid } + context 'when the branch exists' do + context 'when the commit does not exist locally' do + let(:source_branch) { 'new-branch-for-fetch-source-branch' } + let(:source_rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { source_repository.rugged } } + let(:new_oid) { new_commit_edit_old_file(source_rugged).oid } - before do - source_rugged.branches.create(source_branch, new_oid) - end - - it 'writes the ref' do - expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true) - expect(repository.commit(local_ref).sha).to eq(new_oid) - end + before do + source_rugged.branches.create(source_branch, new_oid) end - context 'when the commit exists locally' do - let(:source_branch) { 'master' } - let(:expected_oid) { SeedRepo::LastCommit::ID } - - it 'writes the ref' do - # Sanity check: the commit should already exist - expect(repository.commit(expected_oid)).not_to be_nil - - expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true) - expect(repository.commit(local_ref).sha).to eq(expected_oid) - end + it 'writes the ref' do + expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true) + expect(repository.commit(local_ref).sha).to eq(new_oid) end end - context 'when the branch does not exist' do - let(:source_branch) { 'definitely-not-master' } + context 'when the commit exists locally' do + let(:source_branch) { 'master' } + let(:expected_oid) { SeedRepo::LastCommit::ID } - it 'does not write the ref' do - expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(false) - expect(repository.commit(local_ref)).to be_nil + it 'writes the ref' do + # Sanity check: the commit should already exist + expect(repository.commit(expected_oid)).not_to be_nil + + expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true) + expect(repository.commit(local_ref).sha).to eq(expected_oid) end end end - it_behaves_like '#fetch_source_branch!' + context 'when the branch does not exist' do + let(:source_branch) { 'definitely-not-master' } - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#fetch_source_branch!' + it 'does not write the ref' do + expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(false) + expect(repository.commit(local_ref)).to be_nil + end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index c435f988cdd..f8bf896950e 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -403,46 +403,36 @@ describe Gitlab::Shell do end describe '#create_repository' do - shared_examples '#create_repository' do - let(:repository_storage) { 'default' } - let(:repository_storage_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab.config.repositories.storages[repository_storage].legacy_disk_path - end - end - let(:repo_name) { 'project/path' } - let(:created_path) { File.join(repository_storage_path, repo_name + '.git') } - - after do - FileUtils.rm_rf(created_path) - end - - it 'creates a repository' do - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_truthy - - expect(File.stat(created_path).mode & 0o777).to eq(0o770) - - hooks_path = File.join(created_path, 'hooks') - expect(File.lstat(hooks_path)).to be_symlink - expect(File.realpath(hooks_path)).to eq(gitlab_shell_hooks_path) - end - - it 'returns false when the command fails' do - FileUtils.mkdir_p(File.dirname(created_path)) - # This file will block the creation of the repo's .git directory. That - # should cause #create_repository to fail. - FileUtils.touch(created_path) - - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_falsy + let(:repository_storage) { 'default' } + let(:repository_storage_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages[repository_storage].legacy_disk_path end end + let(:repo_name) { 'project/path' } + let(:created_path) { File.join(repository_storage_path, repo_name + '.git') } - context 'with gitaly' do - it_behaves_like '#create_repository' + after do + FileUtils.rm_rf(created_path) end - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#create_repository' + it 'creates a repository' do + expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_truthy + + expect(File.stat(created_path).mode & 0o777).to eq(0o770) + + hooks_path = File.join(created_path, 'hooks') + expect(File.lstat(hooks_path)).to be_symlink + expect(File.realpath(hooks_path)).to eq(gitlab_shell_hooks_path) + end + + it 'returns false when the command fails' do + FileUtils.mkdir_p(File.dirname(created_path)) + # This file will block the creation of the repo's .git directory. That + # should cause #create_repository to fail. + FileUtils.touch(created_path) + + expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_falsy end end @@ -513,22 +503,12 @@ describe Gitlab::Shell do end end - shared_examples 'fetch_remote' do |gitaly_on| + describe '#fetch_remote' do def fetch_remote(ssh_auth = nil, prune = true) gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', ssh_auth: ssh_auth, prune: prune) end - def expect_gitlab_projects(fail = false, options = {}) - expect(gitlab_projects).to receive(:fetch_remote).with( - 'remote-name', - timeout, - options - ).and_return(!fail) - - allow(gitlab_projects).to receive(:output).and_return('error') if fail - end - - def expect_gitaly_call(fail, options = {}) + def expect_call(fail, options = {}) receive_fetch_remote = if fail receive(:fetch_remote).and_raise(GRPC::NotFound) @@ -539,16 +519,6 @@ describe Gitlab::Shell do expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive_fetch_remote end - if gitaly_on - def expect_call(fail, options = {}) - expect_gitaly_call(fail, options) - end - else - def expect_call(fail, options = {}) - expect_gitlab_projects(fail, options) - end - end - def build_ssh_auth(opts = {}) defaults = { ssh_import?: true, @@ -634,14 +604,6 @@ describe Gitlab::Shell do expect(fetch_remote(ssh_auth)).to be_truthy end end - end - - describe '#fetch_remote local', :skip_gitaly_mock do - it_should_behave_like 'fetch_remote', false - end - - describe '#fetch_remote gitaly' do - it_should_behave_like 'fetch_remote', true context 'gitaly call' do let(:remote_name) { 'remote-name' } @@ -683,25 +645,6 @@ describe Gitlab::Shell do end.to raise_error(Gitlab::Shell::Error, "error") end end - - context 'without gitaly', :disable_gitaly do - it 'returns true when the command succeeds' do - expect(gitlab_projects).to receive(:import_project).with(import_url, timeout) { true } - - result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) - - expect(result).to be_truthy - end - - it 'raises an exception when the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:import_project) { false } - - expect do - gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) - end.to raise_error(Gitlab::Shell::Error, "error") - end - end end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 8fdcfe79fb5..b9e8c6c663a 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -36,22 +36,20 @@ describe Gitlab::Workhorse do allow(described_class).to receive(:git_archive_cache_disabled?).and_return(cache_disabled) end - context 'when Gitaly workhorse_archive feature is enabled' do - it 'sets the header correctly' do - key, command, params = decode_workhorse_header(subject) + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) - expect(key).to eq('Gitlab-Workhorse-Send-Data') - expect(command).to eq('git-archive') - expect(params).to include(gitaly_params) - end + expect(key).to eq('Gitlab-Workhorse-Send-Data') + expect(command).to eq('git-archive') + expect(params).to include(gitaly_params) + end - context 'when archive caching is disabled' do - let(:cache_disabled) { true } + context 'when archive caching is disabled' do + let(:cache_disabled) { true } - it 'tells workhorse not to use the cache' do - _, _, params = decode_workhorse_header(subject) - expect(params).to include({ 'DisableCache' => true }) - end + it 'tells workhorse not to use the cache' do + _, _, params = decode_workhorse_header(subject) + expect(params).to include({ 'DisableCache' => true }) end end From 6b2ebea7dc036c2b21bd47f2955639f9b257b568 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 9 Jul 2018 13:06:12 +0200 Subject: [PATCH 12/38] Added test and used Array() instead of .wrap --- lib/uploaded_file.rb | 2 +- spec/lib/gitlab/middleware/multipart_spec.rb | 27 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/uploaded_file.rb b/lib/uploaded_file.rb index 0172461670b..4b9cb59eab5 100644 --- a/lib/uploaded_file.rb +++ b/lib/uploaded_file.rb @@ -37,7 +37,7 @@ class UploadedFile file_path = File.realpath(params["#{field}.path"]) - paths = Array.wrap(upload_paths) << Dir.tmpdir + paths = Array(upload_paths) << Dir.tmpdir unless self.allowed_path?(file_path, paths.compact) raise InvalidPathError, "insecure path used '#{file_path}'" end diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb index b4837a1689a..f788f8ee276 100644 --- a/spec/lib/gitlab/middleware/multipart_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_spec.rb @@ -75,6 +75,33 @@ describe Gitlab::Middleware::Multipart do it_behaves_like 'multipart upload files' end + it 'allows symlinks for uploads dir' do + Tempfile.open('two-levels') do |tempfile| + symlinked_dir = '/some/dir/uploads' + symlinked_path = File.join(symlinked_dir, File.basename(tempfile.path)) + env = post_env({ 'file' => symlinked_path }, { 'file.name' => original_filename, 'file.path' => symlinked_path }, Gitlab::Workhorse.secret, 'gitlab-workhorse') + + allow(FileUploader).to receive(:root).and_return(symlinked_dir) + allow(UploadedFile).to receive(:allowed_paths).and_return([symlinked_dir, Gitlab.config.uploads.storage_path]) + allow(File).to receive(:realpath).and_call_original + allow(File).to receive(:realpath).with(symlinked_dir).and_return(Dir.tmpdir) + allow(File).to receive(:realpath).with(symlinked_path).and_return(tempfile.path) + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with(symlinked_dir).and_return(true) + + # override Dir.tmpdir because this dir is in the list of allowed paths + # and it would match FileUploader.root path (which in this test is linked + # to /tmp too) + allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir')) + + expect(app).to receive(:call) do |env| + expect(Rack::Request.new(env).params['file']).to be_a(::UploadedFile) + end + + middleware.call(env) + end + end + def post_env(rewritten_fields, params, secret, issuer) token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') Rack::MockRequest.env_for( From bc00803af03147452c12e9e2c7e8f0c0cba86f73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Mon, 9 Jul 2018 13:34:18 +0200 Subject: [PATCH 13/38] Access metadata directly from Object Storage Previously we would pull the file, now, we just stream-it as needed from Object Storage --- app/models/ci/build.rb | 4 +- app/uploaders/gitlab_uploader.rb | 17 ++ app/uploaders/job_artifact_uploader.rb | 8 - .../improve-metadata-access-performance.yml | 5 + lib/gitlab/ci/build/artifacts/metadata.rb | 19 +- lib/gitlab/ci/trace/http_io.rb | 197 ------------------ lib/gitlab/http_io.rb | 193 +++++++++++++++++ .../projects/jobs_controller_spec.rb | 2 +- .../ci/build/artifacts/metadata_spec.rb | 24 ++- .../lib/gitlab/{ci/trace => }/http_io_spec.rb | 2 +- spec/support/http_io/http_io_helpers.rb | 4 +- spec/uploaders/job_artifact_uploader_spec.rb | 2 +- 12 files changed, 258 insertions(+), 219 deletions(-) create mode 100644 changelogs/unreleased/improve-metadata-access-performance.yml delete mode 100644 lib/gitlab/ci/trace/http_io.rb create mode 100644 lib/gitlab/http_io.rb rename spec/lib/gitlab/{ci/trace => }/http_io_spec.rb (99%) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 19949f83351..acf555a5369 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -437,9 +437,9 @@ module Ci end def artifacts_metadata_entry(path, **options) - artifacts_metadata.use_file do |metadata_path| + artifacts_metadata.open do |metadata_stream| metadata = Gitlab::Ci::Build::Artifacts::Metadata.new( - metadata_path, + metadata_stream, path, **options) diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 7919f126075..7636acf255c 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -71,6 +71,23 @@ class GitlabUploader < CarrierWave::Uploader::Base File.join('/', self.class.base_dir, dynamic_segment, filename) end + def open + stream = if file_storage? + File.open(path, "rb") if path + else + ::Gitlab::HttpIO.new(url, cached_size) if url + end + + return unless stream + return stream unless block_given? + + begin + yield(stream) + ensure + stream.close + end + end + private # Designed to be overridden by child uploaders that have a dynamic path diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb index 855cf2fc21c..f6af023e0f9 100644 --- a/app/uploaders/job_artifact_uploader.rb +++ b/app/uploaders/job_artifact_uploader.rb @@ -18,14 +18,6 @@ class JobArtifactUploader < GitlabUploader dynamic_segment end - def open - if file_storage? - File.open(path, "rb") if path - else - ::Gitlab::Ci::Trace::HttpIO.new(url, cached_size) if url - end - end - private def dynamic_segment diff --git a/changelogs/unreleased/improve-metadata-access-performance.yml b/changelogs/unreleased/improve-metadata-access-performance.yml new file mode 100644 index 00000000000..b16fa99dd3b --- /dev/null +++ b/changelogs/unreleased/improve-metadata-access-performance.yml @@ -0,0 +1,5 @@ +--- +title: Access metadata directly from Object Storage +merge_request: +author: +type: performance diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 0bbd60d8ffe..375d8bc1ff5 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -7,14 +7,15 @@ module Gitlab module Artifacts class Metadata ParserError = Class.new(StandardError) + InvalidStreamError = Class.new(StandardError) VERSION_PATTERN = /^[\w\s]+(\d+\.\d+\.\d+)/ INVALID_PATH_PATTERN = %r{(^\.?\.?/)|(/\.?\.?/)} - attr_reader :file, :path, :full_version + attr_reader :stream, :path, :full_version - def initialize(file, path, **opts) - @file, @path, @opts = file, path, opts + def initialize(stream, path, **opts) + @stream, @path, @opts = stream, path, opts @full_version = read_version end @@ -103,7 +104,17 @@ module Gitlab end def gzip(&block) - Zlib::GzipReader.open(@file, &block) + raise InvalidStreamError, "Invalid stream" unless @stream + + # restart gzip reading + @stream.seek(0) + + gz = Zlib::GzipReader.new(@stream) + yield(gz) + rescue Zlib::Error => e + raise InvalidStreamError, e.message + ensure + gz&.finish end end end diff --git a/lib/gitlab/ci/trace/http_io.rb b/lib/gitlab/ci/trace/http_io.rb deleted file mode 100644 index 8788af57a67..00000000000 --- a/lib/gitlab/ci/trace/http_io.rb +++ /dev/null @@ -1,197 +0,0 @@ -## -# This class is compatible with IO class (https://ruby-doc.org/core-2.3.1/IO.html) -# source: https://gitlab.com/snippets/1685610 -module Gitlab - module Ci - class Trace - class HttpIO - BUFFER_SIZE = 128.kilobytes - - InvalidURLError = Class.new(StandardError) - FailedToGetChunkError = Class.new(StandardError) - - attr_reader :uri, :size - attr_reader :tell - attr_reader :chunk, :chunk_range - - alias_method :pos, :tell - - def initialize(url, size) - raise InvalidURLError unless ::Gitlab::UrlSanitizer.valid?(url) - - @uri = URI(url) - @size = size - @tell = 0 - end - - def close - # no-op - end - - def binmode - # no-op - end - - def binmode? - true - end - - def path - nil - end - - def url - @uri.to_s - end - - def seek(pos, where = IO::SEEK_SET) - new_pos = - case where - when IO::SEEK_END - size + pos - when IO::SEEK_SET - pos - when IO::SEEK_CUR - tell + pos - else - -1 - end - - raise 'new position is outside of file' if new_pos < 0 || new_pos > size - - @tell = new_pos - end - - def eof? - tell == size - end - - def each_line - until eof? - line = readline - break if line.nil? - - yield(line) - end - end - - def read(length = nil, outbuf = "") - out = "" - - length ||= size - tell - - until length <= 0 || eof? - data = get_chunk - break if data.empty? - - chunk_bytes = [BUFFER_SIZE - chunk_offset, length].min - chunk_data = data.byteslice(0, chunk_bytes) - - out << chunk_data - @tell += chunk_data.bytesize - length -= chunk_data.bytesize - end - - # If outbuf is passed, we put the output into the buffer. This supports IO.copy_stream functionality - if outbuf - outbuf.slice!(0, outbuf.bytesize) - outbuf << out - end - - out - end - - def readline - out = "" - - until eof? - data = get_chunk - new_line = data.index("\n") - - if !new_line.nil? - out << data[0..new_line] - @tell += new_line + 1 - break - else - out << data - @tell += data.bytesize - end - end - - out - end - - def write(data) - raise NotImplementedError - end - - def truncate(offset) - raise NotImplementedError - end - - def flush - raise NotImplementedError - end - - def present? - true - end - - private - - ## - # The below methods are not implemented in IO class - # - def in_range? - @chunk_range&.include?(tell) - end - - def get_chunk - unless in_range? - response = Net::HTTP.start(uri.hostname, uri.port, proxy_from_env: true, use_ssl: uri.scheme == 'https') do |http| - http.request(request) - end - - raise FailedToGetChunkError unless response.code == '200' || response.code == '206' - - @chunk = response.body.force_encoding(Encoding::BINARY) - @chunk_range = response.content_range - - ## - # Note: If provider does not return content_range, then we set it as we requested - # Provider: minio - # - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 - # - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 - # Provider: AWS - # - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 - # - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 - # Provider: GCS - # - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 - # - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPOK 200 - @chunk_range ||= (chunk_start...(chunk_start + @chunk.bytesize)) - end - - @chunk[chunk_offset..BUFFER_SIZE] - end - - def request - Net::HTTP::Get.new(uri).tap do |request| - request.set_range(chunk_start, BUFFER_SIZE) - end - end - - def chunk_offset - tell % BUFFER_SIZE - end - - def chunk_start - (tell / BUFFER_SIZE) * BUFFER_SIZE - end - - def chunk_end - [chunk_start + BUFFER_SIZE, size].min - end - end - end - end -end diff --git a/lib/gitlab/http_io.rb b/lib/gitlab/http_io.rb new file mode 100644 index 00000000000..ce24817db54 --- /dev/null +++ b/lib/gitlab/http_io.rb @@ -0,0 +1,193 @@ +## +# This class is compatible with IO class (https://ruby-doc.org/core-2.3.1/IO.html) +# source: https://gitlab.com/snippets/1685610 +module Gitlab + class HttpIO + BUFFER_SIZE = 128.kilobytes + + InvalidURLError = Class.new(StandardError) + FailedToGetChunkError = Class.new(StandardError) + + attr_reader :uri, :size + attr_reader :tell + attr_reader :chunk, :chunk_range + + alias_method :pos, :tell + + def initialize(url, size) + raise InvalidURLError unless ::Gitlab::UrlSanitizer.valid?(url) + + @uri = URI(url) + @size = size + @tell = 0 + end + + def close + # no-op + end + + def binmode + # no-op + end + + def binmode? + true + end + + def path + nil + end + + def url + @uri.to_s + end + + def seek(pos, where = IO::SEEK_SET) + new_pos = + case where + when IO::SEEK_END + size + pos + when IO::SEEK_SET + pos + when IO::SEEK_CUR + tell + pos + else + -1 + end + + raise 'new position is outside of file' if new_pos < 0 || new_pos > size + + @tell = new_pos + end + + def eof? + tell == size + end + + def each_line + until eof? + line = readline + break if line.nil? + + yield(line) + end + end + + def read(length = nil, outbuf = "") + out = "" + + length ||= size - tell + + until length <= 0 || eof? + data = get_chunk + break if data.empty? + + chunk_bytes = [BUFFER_SIZE - chunk_offset, length].min + chunk_data = data.byteslice(0, chunk_bytes) + + out << chunk_data + @tell += chunk_data.bytesize + length -= chunk_data.bytesize + end + + # If outbuf is passed, we put the output into the buffer. This supports IO.copy_stream functionality + if outbuf + outbuf.slice!(0, outbuf.bytesize) + outbuf << out + end + + out + end + + def readline + out = "" + + until eof? + data = get_chunk + new_line = data.index("\n") + + if !new_line.nil? + out << data[0..new_line] + @tell += new_line + 1 + break + else + out << data + @tell += data.bytesize + end + end + + out + end + + def write(data) + raise NotImplementedError + end + + def truncate(offset) + raise NotImplementedError + end + + def flush + raise NotImplementedError + end + + def present? + true + end + + private + + ## + # The below methods are not implemented in IO class + # + def in_range? + @chunk_range&.include?(tell) + end + + def get_chunk + unless in_range? + response = Net::HTTP.start(uri.hostname, uri.port, proxy_from_env: true, use_ssl: uri.scheme == 'https') do |http| + http.request(request) + end + + raise FailedToGetChunkError unless response.code == '200' || response.code == '206' + + @chunk = response.body.force_encoding(Encoding::BINARY) + @chunk_range = response.content_range + + ## + # Note: If provider does not return content_range, then we set it as we requested + # Provider: minio + # - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 + # - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 + # Provider: AWS + # - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 + # - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 + # Provider: GCS + # - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206 + # - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPOK 200 + @chunk_range ||= (chunk_start...(chunk_start + @chunk.bytesize)) + end + + @chunk[chunk_offset..BUFFER_SIZE] + end + + def request + Net::HTTP::Get.new(uri).tap do |request| + request.set_range(chunk_start, BUFFER_SIZE) + end + end + + def chunk_offset + tell % BUFFER_SIZE + end + + def chunk_start + (tell / BUFFER_SIZE) * BUFFER_SIZE + end + + def chunk_end + [chunk_start + BUFFER_SIZE, size].min + end + end +end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index e6332a10265..5f53a2d087a 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -245,7 +245,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do end it 'returns a trace' do - expect { get_trace }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError) + expect { get_trace }.to raise_error(Gitlab::HttpIO::FailedToGetChunkError) end end end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 6a52ae01b2f..e327399d82d 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -2,13 +2,21 @@ require 'spec_helper' describe Gitlab::Ci::Build::Artifacts::Metadata do def metadata(path = '', **opts) - described_class.new(metadata_file_path, path, **opts) + described_class.new(metadata_file_stream, path, **opts) end let(:metadata_file_path) do Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz' end + let(:metadata_file_stream) do + File.open(metadata_file_path) if metadata_file_path + end + + after do + metadata_file_stream&.close + end + context 'metadata file exists' do describe '#find_entries! empty string' do subject { metadata('').find_entries! } @@ -86,11 +94,21 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end context 'metadata file does not exist' do - let(:metadata_file_path) { '' } + let(:metadata_file_path) { nil } describe '#find_entries!' do it 'raises error' do - expect { metadata.find_entries! }.to raise_error(Errno::ENOENT) + expect { metadata.find_entries! }.to raise_error(described_class::InvalidStreamError, /Invalid stream/) + end + end + end + + context 'metadata file is invalid' do + let(:metadata_file_path) { Rails.root + 'spec/fixtures/ci_build_artifacts.zip' } + + describe '#find_entries!' do + it 'raises error' do + expect { metadata.find_entries! }.to raise_error(described_class::InvalidStreamError, /not in gzip format/) end end end diff --git a/spec/lib/gitlab/ci/trace/http_io_spec.rb b/spec/lib/gitlab/http_io_spec.rb similarity index 99% rename from spec/lib/gitlab/ci/trace/http_io_spec.rb rename to spec/lib/gitlab/http_io_spec.rb index 5474e2f518c..2c9a73609a9 100644 --- a/spec/lib/gitlab/ci/trace/http_io_spec.rb +++ b/spec/lib/gitlab/http_io_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Trace::HttpIO do +describe Gitlab::HttpIO do include HttpIOHelpers let(:http_io) { described_class.new(url, size) } diff --git a/spec/support/http_io/http_io_helpers.rb b/spec/support/http_io/http_io_helpers.rb index 2c68c2cd9a6..86b254aa67d 100644 --- a/spec/support/http_io/http_io_helpers.rb +++ b/spec/support/http_io/http_io_helpers.rb @@ -54,12 +54,12 @@ module HttpIOHelpers def set_smaller_buffer_size_than(file_size) blocks = (file_size / 128) new_size = (blocks / 2) * 128 - stub_const("Gitlab::Ci::Trace::HttpIO::BUFFER_SIZE", new_size) + stub_const("Gitlab::HttpIO::BUFFER_SIZE", new_size) end def set_larger_buffer_size_than(file_size) blocks = (file_size / 128) new_size = (blocks * 2) * 128 - stub_const("Gitlab::Ci::Trace::HttpIO::BUFFER_SIZE", new_size) + stub_const("Gitlab::HttpIO::BUFFER_SIZE", new_size) end end diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb index 026e4356ed6..d0b14768867 100644 --- a/spec/uploaders/job_artifact_uploader_spec.rb +++ b/spec/uploaders/job_artifact_uploader_spec.rb @@ -55,7 +55,7 @@ describe JobArtifactUploader do end it 'returns http io stream' do - is_expected.to be_a(Gitlab::Ci::Trace::HttpIO) + is_expected.to be_a(Gitlab::HttpIO) end end end From 5a9f23e780222218cc8418fd859669befcaed1b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Mon, 9 Jul 2018 14:18:31 +0200 Subject: [PATCH 14/38] Fix and add specs for testing metadata entry --- app/uploaders/gitlab_uploader.rb | 4 ++ .../projects/jobs_controller_spec.rb | 7 ++- spec/lib/gitlab/http_io_spec.rb | 41 ++++++------ spec/models/ci/build_spec.rb | 54 ++++++++++++++++ spec/requests/api/jobs_spec.rb | 8 ++- spec/support/http_io/http_io_helpers.rb | 56 ++++++----------- spec/uploaders/gitlab_uploader_spec.rb | 62 +++++++++++++++++++ spec/uploaders/job_artifact_uploader_spec.rb | 37 ----------- 8 files changed, 172 insertions(+), 97 deletions(-) diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 7636acf255c..7aa81a8c312 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -71,6 +71,10 @@ class GitlabUploader < CarrierWave::Uploader::Base File.join('/', self.class.base_dir, dynamic_segment, filename) end + def cached_size + size + end + def open stream = if file_storage? File.open(path, "rb") if path diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 5f53a2d087a..38431fb1598 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -225,8 +225,11 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do end context 'when there are no network issues' do + let(:url) { 'http://object-storage/trace' } + let(:file) { expand_fixture_path('trace/sample_trace') } + before do - stub_remote_trace_206 + stub_remote_url_206(url, file) get_trace end @@ -241,7 +244,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'when there is a network issue' do before do - stub_remote_trace_500 + stub_remote_url_500(url) end it 'returns a trace' do diff --git a/spec/lib/gitlab/http_io_spec.rb b/spec/lib/gitlab/http_io_spec.rb index 2c9a73609a9..788bddb8f59 100644 --- a/spec/lib/gitlab/http_io_spec.rb +++ b/spec/lib/gitlab/http_io_spec.rb @@ -4,8 +4,11 @@ describe Gitlab::HttpIO do include HttpIOHelpers let(:http_io) { described_class.new(url, size) } - let(:url) { remote_trace_url } - let(:size) { remote_trace_size } + + let(:url) { 'http://object-storage/trace' } + let(:file_path) { expand_fixture_path('trace/sample_trace') } + let(:file_body) { File.read(file_path).force_encoding(Encoding::BINARY) } + let(:size) { File.size(file_path) } describe '#close' do subject { http_io.close } @@ -86,10 +89,10 @@ describe Gitlab::HttpIO do describe '#each_line' do subject { http_io.each_line } - let(:string_io) { StringIO.new(remote_trace_body) } + let(:string_io) { StringIO.new(file_body) } before do - stub_remote_trace_206 + stub_remote_url_206(url, file_path) end it 'yields lines' do @@ -99,7 +102,7 @@ describe Gitlab::HttpIO do context 'when buckets on GCS' do context 'when BUFFER_SIZE is larger than file size' do before do - stub_remote_trace_200 + stub_remote_url_200(url, file_path) set_larger_buffer_size_than(size) end @@ -117,7 +120,7 @@ describe Gitlab::HttpIO do context 'when there are no network issue' do before do - stub_remote_trace_206 + stub_remote_url_206(url, file_path) end context 'when read whole size' do @@ -129,7 +132,7 @@ describe Gitlab::HttpIO do end it 'reads a trace' do - is_expected.to eq(remote_trace_body) + is_expected.to eq(file_body) end end @@ -139,7 +142,7 @@ describe Gitlab::HttpIO do end it 'reads a trace' do - is_expected.to eq(remote_trace_body) + is_expected.to eq(file_body) end end end @@ -153,7 +156,7 @@ describe Gitlab::HttpIO do end it 'reads a trace' do - is_expected.to eq(remote_trace_body[0, length]) + is_expected.to eq(file_body[0, length]) end end @@ -163,7 +166,7 @@ describe Gitlab::HttpIO do end it 'reads a trace' do - is_expected.to eq(remote_trace_body[0, length]) + is_expected.to eq(file_body[0, length]) end end end @@ -177,7 +180,7 @@ describe Gitlab::HttpIO do end it 'reads a trace' do - is_expected.to eq(remote_trace_body) + is_expected.to eq(file_body) end end @@ -187,7 +190,7 @@ describe Gitlab::HttpIO do end it 'reads a trace' do - is_expected.to eq(remote_trace_body) + is_expected.to eq(file_body) end end end @@ -221,11 +224,11 @@ describe Gitlab::HttpIO do let(:length) { nil } before do - stub_remote_trace_500 + stub_remote_url_500(url) end it 'reads a trace' do - expect { subject }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError) + expect { subject }.to raise_error(Gitlab::HttpIO::FailedToGetChunkError) end end end @@ -233,15 +236,15 @@ describe Gitlab::HttpIO do describe '#readline' do subject { http_io.readline } - let(:string_io) { StringIO.new(remote_trace_body) } + let(:string_io) { StringIO.new(file_body) } before do - stub_remote_trace_206 + stub_remote_url_206(url, file_path) end shared_examples 'all line matching' do it 'reads a line' do - (0...remote_trace_body.lines.count).each do + (0...file_body.lines.count).each do expect(http_io.readline).to eq(string_io.readline) end end @@ -251,11 +254,11 @@ describe Gitlab::HttpIO do let(:length) { nil } before do - stub_remote_trace_500 + stub_remote_url_500(url) end it 'reads a trace' do - expect { subject }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError) + expect { subject }.to raise_error(Gitlab::HttpIO::FailedToGetChunkError) end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 090ca159e08..bf116c07495 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2687,4 +2687,58 @@ describe Ci::Build do end end end + + describe '#artifacts_metadata_entry' do + set(:build) { create(:ci_build, project: project) } + let(:path) { 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' } + + before do + stub_artifacts_object_storage + end + + subject { build.artifacts_metadata_entry(path) } + + context 'when using local storage' do + let!(:metadata) { create(:ci_job_artifact, :metadata, job: build) } + + context 'for existing file' do + it 'does exist' do + is_expected.to be_exists + end + end + + context 'for non-existing file' do + let(:path) { 'invalid-file' } + + it 'does not exist' do + is_expected.not_to be_exists + end + end + end + + context 'when using remote storage' do + include HttpIOHelpers + + let!(:metadata) { create(:ci_job_artifact, :remote_store, :metadata, job: build) } + let(:file_path) { expand_fixture_path('ci_build_artifacts_metadata.gz') } + + before do + stub_remote_url_206(metadata.file.url, file_path) + end + + context 'for existing file' do + it 'does exist' do + is_expected.to be_exists + end + end + + context 'for non-existing file' do + let(:path) { 'invalid-file' } + + it 'does not exist' do + is_expected.not_to be_exists + end + end + end + end end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 50d6f4b4d99..8a2812d1576 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -535,12 +535,14 @@ describe API::Jobs do context 'authorized user' do context 'when trace is in ObjectStorage' do let!(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) } + let(:url) { 'http://object-storage/trace' } + let(:file) { expand_fixture_path('trace/sample_trace') } before do - stub_remote_trace_206 + stub_remote_url_206(url, file_path) allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false } - allow_any_instance_of(JobArtifactUploader).to receive(:url) { remote_trace_url } - allow_any_instance_of(JobArtifactUploader).to receive(:size) { remote_trace_size } + allow_any_instance_of(JobArtifactUploader).to receive(:url) { url } + allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file) } end it 'returns specific job trace' do diff --git a/spec/support/http_io/http_io_helpers.rb b/spec/support/http_io/http_io_helpers.rb index 86b254aa67d..42144870eb5 100644 --- a/spec/support/http_io/http_io_helpers.rb +++ b/spec/support/http_io/http_io_helpers.rb @@ -1,54 +1,38 @@ module HttpIOHelpers - def stub_remote_trace_206 - WebMock.stub_request(:get, remote_trace_url) - .to_return { |request| remote_trace_response(request, 206) } + def stub_remote_url_206(url, file_path) + WebMock.stub_request(:get, url) + .to_return { |request| remote_url_response(file_path, request, 206) } end - def stub_remote_trace_200 - WebMock.stub_request(:get, remote_trace_url) - .to_return { |request| remote_trace_response(request, 200) } + def stub_remote_url_200(url, file_path) + WebMock.stub_request(:get, url) + .to_return { |request| remote_url_response(file_path, request, 200) } end - def stub_remote_trace_500 - WebMock.stub_request(:get, remote_trace_url) + def stub_remote_url_500(url) + WebMock.stub_request(:get, url) .to_return(status: [500, "Internal Server Error"]) end - def remote_trace_url - "http://trace.com/trace" - end - - def remote_trace_response(request, responce_status) + def remote_url_response(file_path, request, response_status) range = request.headers['Range'].match(/bytes=(\d+)-(\d+)/) + body = File.read(file_path).force_encoding(Encoding::BINARY) + size = body.bytesize + { - status: responce_status, - headers: remote_trace_response_headers(responce_status, range[1].to_i, range[2].to_i), - body: range_trace_body(range[1].to_i, range[2].to_i) + status: response_status, + headers: remote_url_response_headers(response_status, range[1].to_i, range[2].to_i, size), + body: body[range[1].to_i..range[2].to_i] } end - def remote_trace_response_headers(responce_status, from, to) - headers = { 'Content-Type' => 'text/plain' } - - if responce_status == 206 - headers.merge('Content-Range' => "bytes #{from}-#{to}/#{remote_trace_size}") + def remote_url_response_headers(response_status, from, to, size) + { 'Content-Type' => 'text/plain' }.tap do |headers| + if response_status == 206 + headers.merge('Content-Range' => "bytes #{from}-#{to}/#{size}") + end end - - headers - end - - def range_trace_body(from, to) - remote_trace_body[from..to] - end - - def remote_trace_body - @remote_trace_body ||= File.read(expand_fixture_path('trace/sample_trace')) - .force_encoding(Encoding::BINARY) - end - - def remote_trace_size - remote_trace_body.bytesize end def set_smaller_buffer_size_than(file_size) diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb index 362f89424d4..44718ed1212 100644 --- a/spec/uploaders/gitlab_uploader_spec.rb +++ b/spec/uploaders/gitlab_uploader_spec.rb @@ -68,4 +68,66 @@ describe GitlabUploader do expect(subject.file.path).to match(/#{subject.cache_dir}/) end end + + describe '#open' do + context 'when trace is stored in File storage' do + context 'when file exists' do + let(:file) do + fixture_file_upload('spec/fixtures/trace/sample_trace', 'text/plain') + end + + before do + subject.store!(file) + end + + it 'returns io stream' do + expect(subject.open).to be_a(IO) + end + + it 'when passing block it yields' do + expect { |b| subject.open(&b) }.to yield_control + end + end + + context 'when file does not exist' do + it 'returns nil' do + expect(subject.open).to be_nil + end + + it 'when passing block it does not yield' do + expect { |b| subject.open(&b) }.not_to yield_control + end + end + end + + context 'when trace is stored in Object storage' do + before do + allow(subject).to receive(:file_storage?) { false } + end + + context 'when file exists' do + before do + allow(subject).to receive(:url) { 'http://object_storage.com/trace' } + end + + it 'returns http io stream' do + expect(subject.open).to be_a(Gitlab::HttpIO) + end + + it 'when passing block it yields' do + expect { |b| subject.open(&b) }.to yield_control.once + end + end + + context 'when file does not exist' do + it 'returns nil' do + expect(subject.open).to be_nil + end + + it 'when passing block it does not yield' do + expect { |b| subject.open(&b) }.not_to yield_control + end + end + end + end end diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb index d0b14768867..3ad5fe7e3b3 100644 --- a/spec/uploaders/job_artifact_uploader_spec.rb +++ b/spec/uploaders/job_artifact_uploader_spec.rb @@ -23,43 +23,6 @@ describe JobArtifactUploader do store_dir: %r[\h{2}/\h{2}/\h{64}/\d{4}_\d{1,2}_\d{1,2}/\d+/\d+\z] end - describe '#open' do - subject { uploader.open } - - context 'when trace is stored in File storage' do - context 'when file exists' do - let(:file) do - fixture_file_upload('spec/fixtures/trace/sample_trace', 'text/plain') - end - - before do - uploader.store!(file) - end - - it 'returns io stream' do - is_expected.to be_a(IO) - end - end - - context 'when file does not exist' do - it 'returns nil' do - is_expected.to be_nil - end - end - end - - context 'when trace is stored in Object storage' do - before do - allow(uploader).to receive(:file_storage?) { false } - allow(uploader).to receive(:url) { 'http://object_storage.com/trace' } - end - - it 'returns http io stream' do - is_expected.to be_a(Gitlab::HttpIO) - end - end - end - context 'file is stored in valid local_path' do let(:file) do fixture_file_upload('spec/fixtures/ci_build_artifacts.zip', 'application/zip') From 612f5e63b3cf76c354e50f4aa6c4163e5465b115 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 28 Mar 2018 10:29:08 +0000 Subject: [PATCH 15/38] Update rubocop to get rid of a warning in other MR --- .rubocop_todo.yml | 6 ----- Gemfile | 4 ++-- Gemfile.lock | 22 ++++++++++++------- Gemfile.rails5.lock | 22 ++++++++++++------- .../line_break_around_conditional_block.rb | 2 ++ 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3e1713f845e..8a1ca6747a8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -199,12 +199,6 @@ Naming/HeredocDelimiterCase: Naming/HeredocDelimiterNaming: Enabled: false -# Offense count: 27 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect. -Performance/HashEachMethods: - Enabled: false - # Offense count: 1 Performance/UnfreezeString: Exclude: diff --git a/Gemfile b/Gemfile index 1fbc240bf6f..e52627dbadd 100644 --- a/Gemfile +++ b/Gemfile @@ -351,9 +351,9 @@ group :development, :test do gem 'spring', '~> 2.0.0' gem 'spring-commands-rspec', '~> 1.0.4' - gem 'gitlab-styles', '~> 2.3', require: false + gem 'gitlab-styles', '~> 2.4', require: false, git: 'https://gitlab.com/gitlab-org/gitlab-styles.git', branch: 'update-to-0.54.0' # Pin these dependencies, otherwise a new rule could break the CI pipelines - gem 'rubocop', '~> 0.52.1' + gem 'rubocop', '~> 0.54.0' gem 'rubocop-rspec', '~> 1.22.1' gem 'scss_lint', '~> 0.56.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index a889c4dc3a0..41d22f3ec89 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,13 @@ +GIT + remote: https://gitlab.com/gitlab-org/gitlab-styles.git + revision: f3250343d3a13a154fe9b2d356440654297b0624 + branch: update-to-0.54.0 + specs: + gitlab-styles (2.4.0) + rubocop (~> 0.54.0) + rubocop-gitlab-security (~> 0.1.0) + rubocop-rspec (~> 1.19) + GEM remote: https://rubygems.org/ specs: @@ -312,10 +322,6 @@ GEM mime-types (>= 1.16) posix-spawn (~> 0.3) gitlab-markup (1.6.4) - gitlab-styles (2.3.2) - rubocop (~> 0.51) - rubocop-gitlab-security (~> 0.1.0) - rubocop-rspec (~> 1.19) gitlab_omniauth-ldap (2.0.4) net-ldap (~> 0.16) omniauth (~> 1.3) @@ -776,9 +782,9 @@ GEM pg rails sqlite3 - rubocop (0.52.1) + rubocop (0.54.0) parallel (~> 1.10) - parser (>= 2.4.0.2, < 3.0) + parser (>= 2.5) powerpack (~> 0.1) rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) @@ -1043,7 +1049,7 @@ DEPENDENCIES gitlab-gollum-lib (~> 4.2) gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.4) - gitlab-styles (~> 2.3) + gitlab-styles (~> 2.4)! gitlab_omniauth-ldap (~> 2.0.4) gon (~> 6.2) google-api-client (~> 0.19.8) @@ -1143,7 +1149,7 @@ DEPENDENCIES rspec-retry (~> 0.4.5) rspec-set (~> 0.1.3) rspec_profiling (~> 0.0.5) - rubocop (~> 0.52.1) + rubocop (~> 0.54.0) rubocop-rspec (~> 1.22.1) ruby-fogbugz (~> 0.2.1) ruby-prof (~> 0.17.0) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 8c46b8c5916..b74cbf41343 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -1,3 +1,13 @@ +GIT + remote: https://gitlab.com/gitlab-org/gitlab-styles.git + revision: f3250343d3a13a154fe9b2d356440654297b0624 + branch: update-to-0.54.0 + specs: + gitlab-styles (2.4.0) + rubocop (~> 0.54.0) + rubocop-gitlab-security (~> 0.1.0) + rubocop-rspec (~> 1.19) + GEM remote: https://rubygems.org/ specs: @@ -315,10 +325,6 @@ GEM mime-types (>= 1.16) posix-spawn (~> 0.3) gitlab-markup (1.6.4) - gitlab-styles (2.3.2) - rubocop (~> 0.51) - rubocop-gitlab-security (~> 0.1.0) - rubocop-rspec (~> 1.19) gitlab_omniauth-ldap (2.0.4) net-ldap (~> 0.16) omniauth (~> 1.3) @@ -785,9 +791,9 @@ GEM pg rails sqlite3 - rubocop (0.52.1) + rubocop (0.54.0) parallel (~> 1.10) - parser (>= 2.4.0.2, < 3.0) + parser (>= 2.5) powerpack (~> 0.1) rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) @@ -1053,7 +1059,7 @@ DEPENDENCIES gitlab-gollum-lib (~> 4.2) gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.4) - gitlab-styles (~> 2.3) + gitlab-styles (~> 2.4)! gitlab_omniauth-ldap (~> 2.0.4) gon (~> 6.2) google-api-client (~> 0.19.8) @@ -1154,7 +1160,7 @@ DEPENDENCIES rspec-retry (~> 0.4.5) rspec-set (~> 0.1.3) rspec_profiling (~> 0.0.5) - rubocop (~> 0.52.1) + rubocop (~> 0.54.0) rubocop-rspec (~> 1.22.1) ruby-fogbugz (~> 0.2.1) ruby-prof (~> 0.17.0) diff --git a/rubocop/cop/line_break_around_conditional_block.rb b/rubocop/cop/line_break_around_conditional_block.rb index 8b6052fee1b..011f2bcf8bf 100644 --- a/rubocop/cop/line_break_around_conditional_block.rb +++ b/rubocop/cop/line_break_around_conditional_block.rb @@ -43,6 +43,8 @@ module RuboCop # end # end class LineBreakAroundConditionalBlock < RuboCop::Cop::Cop + include RangeHelp + MSG = 'Add a line break around conditional blocks' def on_if(node) From 4ee08b77bc5ae11553d59c182ea8292b77699115 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 2 Jul 2018 10:43:06 +0000 Subject: [PATCH 16/38] Updates from `rubocop -a` --- Rakefile | 4 +- .../admin/deploy_keys_controller.rb | 4 +- app/controllers/admin/groups_controller.rb | 2 +- app/controllers/admin/hooks_controller.rb | 4 +- .../admin/identities_controller.rb | 2 +- .../admin/impersonations_controller.rb | 2 +- app/controllers/admin/jobs_controller.rb | 2 +- .../admin/runner_projects_controller.rb | 2 +- app/controllers/admin/runners_controller.rb | 2 +- app/controllers/admin/services_controller.rb | 2 +- app/controllers/admin/users_controller.rb | 2 +- app/controllers/concerns/issuable_actions.rb | 2 +- app/controllers/concerns/lfs_request.rb | 2 +- app/controllers/groups/avatars_controller.rb | 2 +- app/controllers/groups/runners_controller.rb | 2 +- app/controllers/jwt_controller.rb | 4 +- .../notification_settings_controller.rb | 4 +- .../profiles/active_sessions_controller.rb | 2 +- .../profiles/avatars_controller.rb | 2 +- .../profiles/chat_names_controller.rb | 2 +- app/controllers/profiles/emails_controller.rb | 2 +- .../profiles/gpg_keys_controller.rb | 4 +- app/controllers/profiles/keys_controller.rb | 2 +- .../profiles/two_factor_auths_controller.rb | 2 +- .../projects/application_controller.rb | 2 +- .../projects/avatars_controller.rb | 2 +- .../projects/branches_controller.rb | 2 +- .../projects/clusters_controller.rb | 2 +- .../projects/deploy_keys_controller.rb | 2 +- .../projects/environments_controller.rb | 2 +- .../projects/git_http_client_controller.rb | 4 +- .../projects/group_links_controller.rb | 4 +- app/controllers/projects/hooks_controller.rb | 4 +- app/controllers/projects/labels_controller.rb | 4 +- .../projects/lfs_api_controller.rb | 2 +- .../projects/lfs_storage_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- .../projects/milestones_controller.rb | 2 +- .../projects/mirrors_controller.rb | 2 +- .../projects/pipeline_schedules_controller.rb | 2 +- .../projects/releases_controller.rb | 2 +- .../projects/repositories_controller.rb | 2 +- .../projects/runner_projects_controller.rb | 2 +- .../projects/runners_controller.rb | 2 +- .../projects/services_controller.rb | 2 +- .../projects/snippets_controller.rb | 2 +- app/controllers/projects/tags_controller.rb | 2 +- .../projects/templates_controller.rb | 2 +- .../projects/triggers_controller.rb | 2 +- app/controllers/projects/wikis_controller.rb | 4 +- app/controllers/projects_controller.rb | 2 +- app/controllers/sessions_controller.rb | 4 +- .../sherlock/transactions_controller.rb | 2 +- app/controllers/snippets_controller.rb | 2 +- app/models/ci/build.rb | 2 +- app/models/concerns/protected_ref.rb | 2 +- app/models/project_wiki.rb | 2 +- app/models/remote_mirror.rb | 2 +- app/models/user.rb | 4 +- app/models/wiki_page.rb | 4 +- app/services/badges/update_service.rb | 2 +- app/services/commits/change_service.rb | 2 - app/services/issuable_base_service.rb | 2 +- app/services/members/update_service.rb | 2 +- app/services/merge_requests/rebase_service.rb | 2 +- app/services/milestones/update_service.rb | 2 +- app/services/notes/update_service.rb | 2 +- .../notification_recipient_service.rb | 2 - app/services/projects/destroy_service.rb | 2 +- app/services/projects/fork_service.rb | 2 +- app/services/projects/update_service.rb | 2 +- app/services/update_release_service.rb | 2 +- .../object_storage/migrate_uploads_worker.rb | 2 - config/application.rb | 4 +- config/environment.rb | 2 +- config/initializers/active_record_locking.rb | 4 +- lib/api/deploy_keys.rb | 6 +-- lib/api/project_hooks.rb | 2 +- lib/api/services.rb | 4 +- lib/api/users.rb | 2 +- .../filter/markdown_engines/common_mark.rb | 2 +- lib/gitlab/auth/o_auth/user.rb | 2 +- .../add_merge_request_diff_commits_count.rb | 1 - .../archive_legacy_traces.rb | 1 - .../create_fork_network_memberships_range.rb | 1 - .../create_gpg_key_subkeys_from_gpg_keys.rb | 1 - .../background_migration/delete_diff_files.rb | 1 - ...rialize_merge_request_diffs_and_commits.rb | 1 - .../fill_file_store_job_artifact.rb | 1 - .../fill_file_store_lfs_object.rb | 1 - .../background_migration/fill_store_upload.rb | 1 - .../migrate_build_stage.rb | 1 - .../migrate_events_to_push_event_payloads.rb | 1 - .../migrate_system_uploads_to_new_folder.rb | 1 - .../move_personal_snippet_files.rb | 1 - .../normalize_ldap_extern_uids_range.rb | 1 - .../populate_fork_networks_range.rb | 2 +- ..._merge_request_metrics_with_events_data.rb | 3 -- .../populate_untracked_uploads.rb | 2 +- ...populate_untracked_uploads_dependencies.rb | 4 +- .../prepare_untracked_uploads.rb | 2 +- lib/gitlab/ci/config/entry/configurable.rb | 2 +- lib/gitlab/current_settings.rb | 2 +- lib/gitlab/encoding_helper.rb | 2 +- lib/gitlab/git/repository.rb | 2 - lib/gitlab/gpg/commit.rb | 2 +- lib/gitlab/mail_room.rb | 2 +- lib/gitlab/metrics/influx_db.rb | 1 - lib/gitlab/metrics/method_call.rb | 2 - lib/tasks/gitlab/uploads/migrate.rake | 2 +- qa/qa/runtime/release.rb | 2 +- .../projects/forks_controller_spec.rb | 2 +- .../projects/group_links_controller_spec.rb | 2 +- .../projects/imports_controller_spec.rb | 8 ++-- .../merge_requests_controller_spec.rb | 2 +- spec/controllers/projects_controller_spec.rb | 4 +- spec/factories/issues.rb | 2 +- spec/factories/merge_requests.rb | 2 +- spec/features/commits_spec.rb | 6 +-- spec/features/dashboard/projects_spec.rb | 6 +-- .../groups/members/request_access_spec.rb | 2 +- spec/features/issues/issue_sidebar_spec.rb | 2 +- .../user_posts_diff_notes_spec.rb | 2 +- spec/features/profiles/password_spec.rb | 2 +- .../projects/files/user_browses_files_spec.rb | 2 - .../projects/jobs/permissions_spec.rb | 2 +- spec/features/projects/jobs_spec.rb | 10 ++--- .../members/user_requests_access_spec.rb | 2 +- .../projects/pipelines/pipelines_spec.rb | 2 +- spec/features/projects/remote_mirror_spec.rb | 4 +- ...user_activates_slack_notifications_spec.rb | 2 +- ...user_sees_deletion_failure_message_spec.rb | 2 +- spec/features/signed_commits_spec.rb | 2 +- spec/finders/group_projects_finder_spec.rb | 4 +- spec/finders/joined_groups_finder_spec.rb | 2 +- spec/finders/move_to_project_finder_spec.rb | 2 +- spec/finders/personal_projects_finder_spec.rb | 2 +- .../move_personal_snippet_files_spec.rb | 2 +- .../v1/rename_namespaces_spec.rb | 2 +- .../v1/rename_projects_spec.rb | 2 +- .../gpg/invalid_gpg_signature_updater_spec.rb | 4 +- .../issues_moved_to_id_foreign_key_spec.rb | 2 +- ...ters_to_new_clusters_architectures_spec.rb | 2 +- spec/models/ci/build_metadata_spec.rb | 2 +- spec/models/ci/build_spec.rb | 16 ++++---- spec/models/ci/job_artifact_spec.rb | 2 +- ...tch_destroy_dependent_associations_spec.rb | 4 +- spec/models/concerns/mentionable_spec.rb | 6 +-- spec/models/concerns/routable_spec.rb | 2 +- spec/models/environment_spec.rb | 2 +- spec/models/group_spec.rb | 4 +- spec/models/merge_request_spec.rb | 2 +- spec/models/namespace_spec.rb | 4 +- spec/models/notification_setting_spec.rb | 2 +- spec/models/project_feature_spec.rb | 2 +- spec/models/project_spec.rb | 18 ++++----- spec/models/remote_mirror_spec.rb | 8 ++-- spec/models/route_spec.rb | 14 +++---- spec/models/service_spec.rb | 2 +- spec/models/user_spec.rb | 38 +++++++++---------- spec/requests/api/access_requests_spec.rb | 2 +- spec/requests/api/helpers_spec.rb | 2 +- spec/requests/api/notes_spec.rb | 4 +- spec/requests/api/pipeline_schedules_spec.rb | 2 +- spec/requests/api/project_import_spec.rb | 2 +- spec/requests/api/projects_spec.rb | 6 +-- spec/requests/api/tags_spec.rb | 6 +-- spec/services/ci/retry_build_service_spec.rb | 2 +- spec/services/issues/update_service_spec.rb | 4 +- spec/services/members/destroy_service_spec.rb | 4 +- .../conflicts/list_service_spec.rb | 2 +- .../merge_requests/merge_service_spec.rb | 2 +- spec/services/notification_service_spec.rb | 2 +- spec/services/projects/fork_service_spec.rb | 2 +- .../projects/update_pages_service_spec.rb | 16 ++++---- .../reset_project_cache_service_spec.rb | 2 +- spec/services/system_hooks_service_spec.rb | 2 +- spec/spec_helper.rb | 2 +- .../api/repositories_shared_context.rb | 2 +- .../api/time_tracking_shared_examples.rb | 8 ++-- spec/support/generate-seed-repo-rb | 2 +- spec/support/helpers/jira_service_helper.rb | 2 +- ...attermost_notifications_shared_examples.rb | 18 ++++----- .../projects/imports/new.html.haml_spec.rb | 2 +- .../merge_requests/show.html.haml_spec.rb | 2 +- spec/workers/repository_import_worker_spec.rb | 4 +- ...sitory_update_remote_mirror_worker_spec.rb | 10 ++--- spec/workers/stuck_import_jobs_worker_spec.rb | 4 +- 188 files changed, 279 insertions(+), 309 deletions(-) diff --git a/Rakefile b/Rakefile index 85fff2d51eb..de0d6695c7b 100755 --- a/Rakefile +++ b/Rakefile @@ -2,9 +2,9 @@ # Add your own tasks in files placed in lib/tasks ending in .rake, # for example lib/tasks/capistrano.rake, and they will automatically be available to Rake. -require File.expand_path('../config/application', __FILE__) +require File.expand_path('config/application', __dir__) -relative_url_conf = File.expand_path('../config/initializers/relative_url', __FILE__) +relative_url_conf = File.expand_path('config/initializers/relative_url', __dir__) require relative_url_conf if File.exist?("#{relative_url_conf}.rb") Gitlab::Application.load_tasks diff --git a/app/controllers/admin/deploy_keys_controller.rb b/app/controllers/admin/deploy_keys_controller.rb index b0c4c31cffc..5c2025c1988 100644 --- a/app/controllers/admin/deploy_keys_controller.rb +++ b/app/controllers/admin/deploy_keys_controller.rb @@ -22,7 +22,7 @@ class Admin::DeployKeysController < Admin::ApplicationController end def update - if deploy_key.update_attributes(update_params) + if deploy_key.update(update_params) flash[:notice] = 'Deploy key was successfully updated.' redirect_to admin_deploy_keys_path else @@ -34,7 +34,7 @@ class Admin::DeployKeysController < Admin::ApplicationController deploy_key.destroy respond_to do |format| - format.html { redirect_to admin_deploy_keys_path, status: 302 } + format.html { redirect_to admin_deploy_keys_path, status: :found } format.json { head :ok } end end diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 96b7bc65ac9..d7a5b745d3f 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -39,7 +39,7 @@ class Admin::GroupsController < Admin::ApplicationController end def update - if @group.update_attributes(group_params) + if @group.update(group_params) redirect_to [:admin, @group], notice: 'Group was successfully updated.' else render "edit" diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index 6944857bd33..a98c355c7ba 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -23,7 +23,7 @@ class Admin::HooksController < Admin::ApplicationController end def update - if hook.update_attributes(hook_params) + if hook.update(hook_params) flash[:notice] = 'System hook was successfully updated.' redirect_to admin_hooks_path else @@ -34,7 +34,7 @@ class Admin::HooksController < Admin::ApplicationController def destroy hook.destroy - redirect_to admin_hooks_path, status: 302 + redirect_to admin_hooks_path, status: :found end def test diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb index 43b4e3a2cc3..ceb45865804 100644 --- a/app/controllers/admin/identities_controller.rb +++ b/app/controllers/admin/identities_controller.rb @@ -25,7 +25,7 @@ class Admin::IdentitiesController < Admin::ApplicationController end def update - if @identity.update_attributes(identity_params) + if @identity.update(identity_params) RepairLdapBlockedUserService.new(@user).execute redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.' else diff --git a/app/controllers/admin/impersonations_controller.rb b/app/controllers/admin/impersonations_controller.rb index 39dbf85f6c0..d2f947d2c66 100644 --- a/app/controllers/admin/impersonations_controller.rb +++ b/app/controllers/admin/impersonations_controller.rb @@ -11,7 +11,7 @@ class Admin::ImpersonationsController < Admin::ApplicationController session[:impersonator_id] = nil - redirect_to admin_user_path(original_user), status: 302 + redirect_to admin_user_path(original_user), status: :found end private diff --git a/app/controllers/admin/jobs_controller.rb b/app/controllers/admin/jobs_controller.rb index ae7a7f6279c..ac1ae0f16b3 100644 --- a/app/controllers/admin/jobs_controller.rb +++ b/app/controllers/admin/jobs_controller.rb @@ -20,6 +20,6 @@ class Admin::JobsController < Admin::ApplicationController def cancel_all Ci::Build.running_or_pending.each(&:cancel) - redirect_to admin_jobs_path, status: 303 + redirect_to admin_jobs_path, status: :see_other end end diff --git a/app/controllers/admin/runner_projects_controller.rb b/app/controllers/admin/runner_projects_controller.rb index 7aba77d8129..51d5799cd89 100644 --- a/app/controllers/admin/runner_projects_controller.rb +++ b/app/controllers/admin/runner_projects_controller.rb @@ -16,7 +16,7 @@ class Admin::RunnerProjectsController < Admin::ApplicationController runner = rp.runner rp.destroy - redirect_to admin_runner_path(runner), status: 302 + redirect_to admin_runner_path(runner), status: :found end private diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index 4b01904f2a1..6c76c55a9d4 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -28,7 +28,7 @@ class Admin::RunnersController < Admin::ApplicationController def destroy @runner.destroy - redirect_to admin_runners_path, status: 302 + redirect_to admin_runners_path, status: :found end def resume diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb index a7025b62ad7..e70aa549140 100644 --- a/app/controllers/admin/services_controller.rb +++ b/app/controllers/admin/services_controller.rb @@ -16,7 +16,7 @@ class Admin::ServicesController < Admin::ApplicationController end def update - if service.update_attributes(service_params[:service]) + if service.update(service_params[:service]) PropagateServiceTemplateWorker.perform_async(service.id) if service.active? redirect_to admin_application_settings_services_path, diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 653f3dfffc4..a51a8c3ed4a 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -163,7 +163,7 @@ class Admin::UsersController < Admin::ApplicationController format.json { head :ok } else format.html { redirect_back_or_admin_user(alert: 'There was an error removing the e-mail.') } - format.json { render json: 'There was an error removing the e-mail.', status: 400 } + format.json { render json: 'There was an error removing the e-mail.', status: :bad_request } end end end diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index ba510968684..37e03d70b6f 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -127,7 +127,7 @@ module IssuableActions errors: [ "Someone edited this #{issuable.human_class_name} at the same time you did. Please refresh your browser and make sure your changes will not unintentionally remove theirs." ] - }, status: 409 + }, status: :conflict end end end diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb index 5e4e8a87153..79ee5b2f91e 100644 --- a/app/controllers/concerns/lfs_request.rb +++ b/app/controllers/concerns/lfs_request.rb @@ -27,7 +27,7 @@ module LfsRequest message: 'Git LFS is not enabled on this GitLab server, contact your admin.', documentation_url: help_url }, - status: 501 + status: :not_implemented ) end diff --git a/app/controllers/groups/avatars_controller.rb b/app/controllers/groups/avatars_controller.rb index cc5ba5878f8..35a61b359c8 100644 --- a/app/controllers/groups/avatars_controller.rb +++ b/app/controllers/groups/avatars_controller.rb @@ -7,6 +7,6 @@ class Groups::AvatarsController < Groups::ApplicationController @group.remove_avatar! @group.save - redirect_to edit_group_path(@group), status: 302 + redirect_to edit_group_path(@group), status: :found end end diff --git a/app/controllers/groups/runners_controller.rb b/app/controllers/groups/runners_controller.rb index 78992ec7f46..1036b4e6ed3 100644 --- a/app/controllers/groups/runners_controller.rb +++ b/app/controllers/groups/runners_controller.rb @@ -23,7 +23,7 @@ class Groups::RunnersController < Groups::ApplicationController def destroy @runner.destroy - redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), status: 302 + redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), status: :found end def resume diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 67057b5b126..3cb9e46b548 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -41,7 +41,7 @@ class JwtController < ApplicationController "You must use a personal access token with 'api' scope for Git over HTTP.\n" \ "You can generate one at #{profile_personal_access_tokens_url}" } ] - }, status: 401 + }, status: :unauthorized end def render_unauthorized @@ -50,7 +50,7 @@ class JwtController < ApplicationController { code: 'UNAUTHORIZED', message: 'HTTP Basic: Access denied' } ] - }, status: 401 + }, status: :unauthorized end def auth_params diff --git a/app/controllers/notification_settings_controller.rb b/app/controllers/notification_settings_controller.rb index 8ec4bb1233f..ed20302487c 100644 --- a/app/controllers/notification_settings_controller.rb +++ b/app/controllers/notification_settings_controller.rb @@ -5,14 +5,14 @@ class NotificationSettingsController < ApplicationController return render_404 unless can_read?(resource) @notification_setting = current_user.notification_settings_for(resource) - @saved = @notification_setting.update_attributes(notification_setting_params) + @saved = @notification_setting.update(notification_setting_params) render_response end def update @notification_setting = current_user.notification_settings.find(params[:id]) - @saved = @notification_setting.update_attributes(notification_setting_params) + @saved = @notification_setting.update(notification_setting_params) render_response end diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb index f0cdc228366..f1e77d68acd 100644 --- a/app/controllers/profiles/active_sessions_controller.rb +++ b/app/controllers/profiles/active_sessions_controller.rb @@ -7,7 +7,7 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController ActiveSession.destroy(current_user, params[:id]) respond_to do |format| - format.html { redirect_to profile_active_sessions_url, status: 302 } + format.html { redirect_to profile_active_sessions_url, status: :found } format.js { head :ok } end end diff --git a/app/controllers/profiles/avatars_controller.rb b/app/controllers/profiles/avatars_controller.rb index 39b9f8a84d1..4f030ded80f 100644 --- a/app/controllers/profiles/avatars_controller.rb +++ b/app/controllers/profiles/avatars_controller.rb @@ -4,6 +4,6 @@ class Profiles::AvatarsController < Profiles::ApplicationController Users::UpdateService.new(current_user, user: @user).execute { |user| user.remove_avatar! } - redirect_to profile_path, status: 302 + redirect_to profile_path, status: :found end end diff --git a/app/controllers/profiles/chat_names_controller.rb b/app/controllers/profiles/chat_names_controller.rb index 2353f0840d6..a186c5f36a8 100644 --- a/app/controllers/profiles/chat_names_controller.rb +++ b/app/controllers/profiles/chat_names_controller.rb @@ -39,7 +39,7 @@ class Profiles::ChatNamesController < Profiles::ApplicationController flash[:alert] = "Could not delete chat nickname #{@chat_name.chat_name}." end - redirect_to profile_chat_names_path, status: 302 + redirect_to profile_chat_names_path, status: :found end private diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb index bbd7ba49d77..a39824ec9c8 100644 --- a/app/controllers/profiles/emails_controller.rb +++ b/app/controllers/profiles/emails_controller.rb @@ -19,7 +19,7 @@ class Profiles::EmailsController < Profiles::ApplicationController Emails::DestroyService.new(current_user, user: current_user).execute(@email) respond_to do |format| - format.html { redirect_to profile_emails_url, status: 302 } + format.html { redirect_to profile_emails_url, status: :found } format.js { head :ok } end end diff --git a/app/controllers/profiles/gpg_keys_controller.rb b/app/controllers/profiles/gpg_keys_controller.rb index 38e3eacd229..c32507756e8 100644 --- a/app/controllers/profiles/gpg_keys_controller.rb +++ b/app/controllers/profiles/gpg_keys_controller.rb @@ -21,7 +21,7 @@ class Profiles::GpgKeysController < Profiles::ApplicationController @gpg_key.destroy respond_to do |format| - format.html { redirect_to profile_gpg_keys_url, status: 302 } + format.html { redirect_to profile_gpg_keys_url, status: :found } format.js { head :ok } end end @@ -30,7 +30,7 @@ class Profiles::GpgKeysController < Profiles::ApplicationController @gpg_key.revoke respond_to do |format| - format.html { redirect_to profile_gpg_keys_url, status: 302 } + format.html { redirect_to profile_gpg_keys_url, status: :found } format.js { head :ok } end end diff --git a/app/controllers/profiles/keys_controller.rb b/app/controllers/profiles/keys_controller.rb index 12a6cd11f80..6035258667e 100644 --- a/app/controllers/profiles/keys_controller.rb +++ b/app/controllers/profiles/keys_controller.rb @@ -26,7 +26,7 @@ class Profiles::KeysController < Profiles::ApplicationController Keys::DestroyService.new(current_user).execute(@key) respond_to do |format| - format.html { redirect_to profile_keys_url, status: 302 } + format.html { redirect_to profile_keys_url, status: :found } format.js { head :ok } end end diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index aa9789f8a0f..29ff18a1219 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -78,7 +78,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController def destroy current_user.disable_two_factor! - redirect_to profile_account_path, status: 302 + redirect_to profile_account_path, status: :found end def skip diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 5ab6d103c89..b4f814fd3a4 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -61,7 +61,7 @@ class Projects::ApplicationController < ApplicationController def require_non_empty_project # Be sure to return status code 303 to avoid a double DELETE: # http://api.rubyonrails.org/classes/ActionController/Redirecting.html - redirect_to project_path(@project), status: 303 if @project.empty_repo? + redirect_to project_path(@project), status: :see_other if @project.empty_repo? end def require_branch_head diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb index 21a403f3765..a13d552dbd8 100644 --- a/app/controllers/projects/avatars_controller.rb +++ b/app/controllers/projects/avatars_controller.rb @@ -21,6 +21,6 @@ class Projects::AvatarsController < Projects::ApplicationController @project.save - redirect_to edit_project_path(@project), status: 302 + redirect_to edit_project_path(@project), status: :found end end diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index cd7250b10fc..d1dc9fe9600 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -98,7 +98,7 @@ class Projects::BranchesController < Projects::ApplicationController flash_type = result[:status] == :error ? :alert : :notice flash[flash_type] = result[:message] - redirect_to project_branches_path(@project), status: 303 + redirect_to project_branches_path(@project), status: :see_other end format.js { render nothing: true, status: result[:return_code] } diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 62193257940..358fe59618b 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -62,7 +62,7 @@ class Projects::ClustersController < Projects::ApplicationController def destroy if cluster.destroy flash[:notice] = _('Kubernetes cluster integration was successfully removed.') - redirect_to project_clusters_path(project), status: 302 + redirect_to project_clusters_path(project), status: :found else flash[:notice] = _('Kubernetes cluster integration was not removed.') render :show diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index f43ef2e5f2f..06739d8fd4a 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -35,7 +35,7 @@ class Projects::DeployKeysController < Projects::ApplicationController end def update - if deploy_key.update_attributes(update_params) + if deploy_key.update(update_params) flash[:notice] = 'Deploy key was successfully updated.' redirect_to_repository_settings(@project) else diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 27b7425b965..395c5336ad5 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -116,7 +116,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController set_workhorse_internal_api_content_type render json: Gitlab::Workhorse.terminal_websocket(terminal) else - render text: 'Not found', status: 404 + render text: 'Not found', status: :not_found end end diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 07249fe3182..a52814e6e52 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -53,7 +53,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController end send_challenges - render plain: "HTTP Basic: Access denied\n", status: 401 + render plain: "HTTP Basic: Access denied\n", status: :unauthorized rescue Gitlab::Auth::MissingPersonalAccessTokenError render_missing_personal_access_token end @@ -83,7 +83,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController render plain: "HTTP Basic: Access denied\n" \ "You must use a personal access token with 'api' scope for Git over HTTP.\n" \ "You can generate one at #{profile_personal_access_tokens_url}", - status: 401 + status: :unauthorized end def repository diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb index f58ee3e9109..bc5f38f3c2b 100644 --- a/app/controllers/projects/group_links_controller.rb +++ b/app/controllers/projects/group_links_controller.rb @@ -24,7 +24,7 @@ class Projects::GroupLinksController < Projects::ApplicationController def update @group_link = @project.project_group_links.find(params[:id]) - @group_link.update_attributes(group_link_params) + @group_link.update(group_link_params) end def destroy @@ -34,7 +34,7 @@ class Projects::GroupLinksController < Projects::ApplicationController respond_to do |format| format.html do - redirect_to project_project_members_path(project), status: 302 + redirect_to project_project_members_path(project), status: :found end format.js { head :ok } end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 6800d742b0a..2da2aad9b33 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -29,7 +29,7 @@ class Projects::HooksController < Projects::ApplicationController end def update - if hook.update_attributes(hook_params) + if hook.update(hook_params) flash[:notice] = 'Hook was successfully updated.' redirect_to project_settings_integrations_path(@project) else @@ -48,7 +48,7 @@ class Projects::HooksController < Projects::ApplicationController def destroy hook.destroy - redirect_to project_settings_integrations_path(@project), status: 302 + redirect_to project_settings_integrations_path(@project), status: :found end private diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 91016f6494e..21d3c918581 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -39,7 +39,7 @@ class Projects::LabelsController < Projects::ApplicationController else respond_to do |format| format.html { render :new } - format.json { render json: { message: @label.errors.messages }, status: 400 } + format.json { render json: { message: @label.errors.messages }, status: :bad_request } end end end @@ -115,7 +115,7 @@ class Projects::LabelsController < Projects::ApplicationController flash[:notice] = "#{@label.title} promoted to group label.".html_safe respond_to do |format| format.html do - redirect_to(project_labels_path(@project), status: 303) + redirect_to(project_labels_path(@project), status: :see_other) end format.json do render json: { url: project_labels_path(@project) } diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb index 3f4962b543d..c64ccc3d473 100644 --- a/app/controllers/projects/lfs_api_controller.rb +++ b/app/controllers/projects/lfs_api_controller.rb @@ -25,7 +25,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController message: 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.', documentation_url: "#{Gitlab.config.gitlab.url}/help" }, - status: 501 + status: :not_implemented ) end diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index 45c98d60822..dd7e673ec75 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -28,7 +28,7 @@ class Projects::LfsStorageController < Projects::GitHttpClientController if store_file!(oid, size) head 200 else - render plain: 'Unprocessable entity', status: 422 + render plain: 'Unprocessable entity', status: :unprocessable_entity end rescue ActiveRecord::RecordInvalid render_lfs_forbidden diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a7c5f858c42..1ad2e93c85f 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -227,7 +227,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo def rebase RebaseWorker.perform_async(@merge_request.id, current_user.id) - render nothing: true, status: 200 + render nothing: true, status: :ok end protected diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index 594563d1f6f..5e86ec93f34 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -96,7 +96,7 @@ class Projects::MilestonesController < Projects::ApplicationController Milestones::DestroyService.new(project, current_user).execute(milestone) respond_to do |format| - format.html { redirect_to namespace_project_milestones_path, status: 303 } + format.html { redirect_to namespace_project_milestones_path, status: :see_other } format.js { head :ok } end end diff --git a/app/controllers/projects/mirrors_controller.rb b/app/controllers/projects/mirrors_controller.rb index 5698ff4e706..3b24d231f3d 100644 --- a/app/controllers/projects/mirrors_controller.rb +++ b/app/controllers/projects/mirrors_controller.rb @@ -13,7 +13,7 @@ class Projects::MirrorsController < Projects::ApplicationController end def update - if project.update_attributes(mirror_params) + if project.update(mirror_params) flash[:notice] = 'Mirroring settings were successfully updated.' else flash[:alert] = project.errors.full_messages.join(', ').html_safe diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index fa258f3d9af..aeda7b3edf5 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -64,7 +64,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController def destroy if schedule.destroy - redirect_to pipeline_schedules_path(@project), status: 302 + redirect_to pipeline_schedules_path(@project), status: :found else redirect_to pipeline_schedules_path(@project), status: :forbidden, diff --git a/app/controllers/projects/releases_controller.rb b/app/controllers/projects/releases_controller.rb index 3e0a530fdb9..19e09b3af6f 100644 --- a/app/controllers/projects/releases_controller.rb +++ b/app/controllers/projects/releases_controller.rb @@ -14,7 +14,7 @@ class Projects::ReleasesController < Projects::ApplicationController # it exists only to save a description to each Tag. # If description is empty we should destroy the existing record. if release_params[:description].present? - release.update_attributes(release_params) + release.update(release_params) else release.destroy end diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index d01f324e6fd..ecb2ece7532 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -24,7 +24,7 @@ class Projects::RepositoriesController < Projects::ApplicationController send_git_archive @repository, ref: @ref, format: params[:format], append_sha: append_sha rescue => ex logger.error("#{self.class.name}: #{ex}") - return git_not_found! + git_not_found! end def assign_archive_vars diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb index a080724634b..c098c82081e 100644 --- a/app/controllers/projects/runner_projects_controller.rb +++ b/app/controllers/projects/runner_projects_controller.rb @@ -21,6 +21,6 @@ class Projects::RunnerProjectsController < Projects::ApplicationController runner_project = project.runner_projects.find(params[:id]) runner_project.destroy - redirect_to project_runners_path(project), status: 302 + redirect_to project_runners_path(project), status: :found end end diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb index bef94cea989..cc7cce887bf 100644 --- a/app/controllers/projects/runners_controller.rb +++ b/app/controllers/projects/runners_controller.rb @@ -24,7 +24,7 @@ class Projects::RunnersController < Projects::ApplicationController @runner.destroy end - redirect_to project_runners_path(@project), status: 302 + redirect_to project_runners_path(@project), status: :found end def resume diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 690596b12db..d55046047ae 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -34,7 +34,7 @@ class Projects::ServicesController < Projects::ApplicationController private def service_test_response - if @service.update_attributes(service_params[:service]) + if @service.update(service_params[:service]) data = @service.test_data(project, current_user) outcome = @service.test(data) diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index 208a1d19862..f742d7edf83 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -82,7 +82,7 @@ class Projects::SnippetsController < Projects::ApplicationController @snippet.destroy - redirect_to project_snippets_path(@project), status: 302 + redirect_to project_snippets_path(@project), status: :found end protected diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index b62d7d9b7c5..b17753222a0 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -50,7 +50,7 @@ class Projects::TagsController < Projects::ApplicationController respond_to do |format| if result[:status] == :success format.html do - redirect_to project_tags_path(@project), status: 303 + redirect_to project_tags_path(@project), status: :see_other end format.js diff --git a/app/controllers/projects/templates_controller.rb b/app/controllers/projects/templates_controller.rb index 694b468c8d3..52d6fb82093 100644 --- a/app/controllers/projects/templates_controller.rb +++ b/app/controllers/projects/templates_controller.rb @@ -14,6 +14,6 @@ class Projects::TemplatesController < Projects::ApplicationController def get_template_class template_types = { issue: Gitlab::Template::IssueTemplate, merge_request: Gitlab::Template::MergeRequestTemplate }.with_indifferent_access @template_type = template_types[params[:template_type]] - render json: [], status: 404 unless @template_type + render json: [], status: :not_found unless @template_type end end diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb index e04145dd0b3..6f3de43f85a 100644 --- a/app/controllers/projects/triggers_controller.rb +++ b/app/controllers/projects/triggers_controller.rb @@ -50,7 +50,7 @@ class Projects::TriggersController < Projects::ApplicationController flash[:alert] = "Could not remove the trigger." end - redirect_to project_settings_ci_cd_path(@project), status: 302 + redirect_to project_settings_ci_cd_path(@project), status: :found end private diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index aa844e94d89..658b6ee5820 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -120,7 +120,7 @@ class Projects::WikisController < Projects::ApplicationController rescue ProjectWiki::CouldNotCreateWikiError flash[:notice] = "Could not create Wiki Repository at this time. Please try again later." redirect_to project_path(@project) - return false + false end def wiki_params @@ -129,7 +129,7 @@ class Projects::WikisController < Projects::ApplicationController def build_page(args) WikiPage.new(@project_wiki).tap do |page| - page.update_attributes(args) + page.update(args) end end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index f2abe27f60e..9d1c44db137 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -133,7 +133,7 @@ class ProjectsController < Projects::ApplicationController ::Projects::DestroyService.new(@project, current_user, {}).async_execute flash[:notice] = _("Project '%{project_name}' is in the process of being deleted.") % { project_name: @project.full_name } - redirect_to dashboard_projects_path, status: 302 + redirect_to dashboard_projects_path, status: :found rescue Projects::DestroyService::DestroyError => ex redirect_to edit_project_path(@project), status: 302, alert: ex.message end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 1de6ae24622..9dd652206fe 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -32,8 +32,8 @@ class SessionsController < Devise::SessionsController super do |resource| # User has successfully signed in, so clear any unused reset token if resource.reset_password_token.present? - resource.update_attributes(reset_password_token: nil, - reset_password_sent_at: nil) + resource.update(reset_password_token: nil, + reset_password_sent_at: nil) end # hide the signed-in notification diff --git a/app/controllers/sherlock/transactions_controller.rb b/app/controllers/sherlock/transactions_controller.rb index cb6c3a7cd98..ae4953c3259 100644 --- a/app/controllers/sherlock/transactions_controller.rb +++ b/app/controllers/sherlock/transactions_controller.rb @@ -13,7 +13,7 @@ module Sherlock def destroy_all Gitlab::Sherlock.collection.clear - redirect_to :back, status: 302 + redirect_to :back, status: :found end end end diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 3d51520ddf4..1d6d0943674 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -89,7 +89,7 @@ class SnippetsController < ApplicationController @snippet.destroy - redirect_to snippets_path, status: 302 + redirect_to snippets_path, status: :found end protected diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 19949f83351..44103e3bc4f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -371,7 +371,7 @@ module Ci def update_coverage coverage = trace.extract_coverage(coverage_regex) - update_attributes(coverage: coverage) if coverage.present? + update(coverage: coverage) if coverage.present? end def parse_trace_sections! diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index 94eef4ff7cd..dbe8d31de37 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -23,7 +23,7 @@ module ProtectedRef # If we don't `protected_branch` or `protected_tag` would be empty and # `project` cannot be delegated to it, which in turn would cause validations # to fail. - has_many :"#{type}_access_levels", inverse_of: self.model_name.singular # rubocop:disable Cop/ActiveRecordDependent + has_many :"#{type}_access_levels", inverse_of: self.model_name.singular validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." } diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index a6f94b3e3b0..fc868c3ebb7 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -107,7 +107,7 @@ class ProjectWiki update_project_activity rescue Gitlab::Git::Wiki::DuplicatePageError => e @error_message = "Duplicate page: #{e.message}" - return false + false end def update_page(page, content:, title: nil, format: :markdown, message: nil) diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index c4b5dd2dc96..976b501e297 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -57,7 +57,7 @@ class RemoteMirror < ActiveRecord::Base Gitlab::Metrics.add_event(:remote_mirrors_finished, path: remote_mirror.project.full_path) timestamp = Time.now - remote_mirror.update_attributes!( + remote_mirror.update!( last_update_at: timestamp, last_successful_update_at: timestamp, last_error: nil ) end diff --git a/app/models/user.rb b/app/models/user.rb index 27a5d0278b7..47542ab122a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -496,7 +496,7 @@ class User < ActiveRecord::Base def disable_two_factor! transaction do - update_attributes( + update( otp_required_for_login: false, encrypted_otp_secret: nil, encrypted_otp_secret_iv: nil, @@ -1053,7 +1053,7 @@ class User < ActiveRecord::Base return @global_notification_setting if defined?(@global_notification_setting) @global_notification_setting = notification_settings.find_or_initialize_by(source: nil) - @global_notification_setting.update_attributes(level: NotificationSetting.levels[DEFAULT_NOTIFICATION_LEVEL]) unless @global_notification_setting.persisted? + @global_notification_setting.update(level: NotificationSetting.levels[DEFAULT_NOTIFICATION_LEVEL]) unless @global_notification_setting.persisted? @global_notification_setting end diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index cde79b95062..6cd3993a56d 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -190,7 +190,7 @@ class WikiPage # Returns the String SHA1 of the newly created page # or False if the save was unsuccessful. def create(attrs = {}) - update_attributes(attrs) + update(attrs) save(page_details: title) do wiki.create_page(title, content, format, message) @@ -216,7 +216,7 @@ class WikiPage raise PageChangedError end - update_attributes(attrs) + update(attrs) if title_changed? page_details = title diff --git a/app/services/badges/update_service.rb b/app/services/badges/update_service.rb index 7ca84b5df31..495a4a2c99d 100644 --- a/app/services/badges/update_service.rb +++ b/app/services/badges/update_service.rb @@ -3,7 +3,7 @@ module Badges # returns the updated badge def execute(badge) if params.present? - badge.update_attributes(params) + badge.update(params) end badge diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index b9d0173a2d0..1ce6ab36cbf 100644 --- a/app/services/commits/change_service.rb +++ b/app/services/commits/change_service.rb @@ -13,8 +13,6 @@ module Commits # rubocop:disable GitlabSecurity/PublicSend message = @commit.public_send(:"#{action}_message", current_user) - - # rubocop:disable GitlabSecurity/PublicSend repository.public_send( action, current_user, diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 683f64e82ad..5e06e0c61cf 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -130,7 +130,7 @@ class IssuableBaseService < BaseService def create_issuable(issuable, attributes, label_ids:) issuable.with_transaction_returning_status do if issuable.save - issuable.update_attributes(label_ids: label_ids) + issuable.update(label_ids: label_ids) end end end diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb index 48b3d59f7bd..cb19cf01dd7 100644 --- a/app/services/members/update_service.rb +++ b/app/services/members/update_service.rb @@ -6,7 +6,7 @@ module Members old_access_level = member.human_access - if member.update_attributes(params) + if member.update(params) after_execute(action: permission, old_access_level: old_access_level, member: member) end diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index 5b4bc86b9ba..c741e913860 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -26,7 +26,7 @@ module MergeRequests Gitlab::GitLogger.info("#{log_prefix} rebased to #{rebase_sha}") - merge_request.update_attributes(rebase_commit_sha: rebase_sha) + merge_request.update(rebase_commit_sha: rebase_sha) Gitlab::GitLogger.info("#{log_prefix} rebase SHA saved: #{rebase_sha}") diff --git a/app/services/milestones/update_service.rb b/app/services/milestones/update_service.rb index 31b441ed476..74edbf9b41d 100644 --- a/app/services/milestones/update_service.rb +++ b/app/services/milestones/update_service.rb @@ -11,7 +11,7 @@ module Milestones end if params.present? - milestone.update_attributes(params.except(:state_event)) + milestone.update(params.except(:state_event)) end milestone diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index 75fd08ea0a9..e16ef398184 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -5,7 +5,7 @@ module Notes old_mentioned_users = note.mentioned_users.to_a - note.update_attributes(params.merge(updated_by: current_user)) + note.update(params.merge(updated_by: current_user)) note.create_new_cross_references!(current_user) if note.previous_changes.include?('note') diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 4fa38665abc..d3447aa9951 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -43,8 +43,6 @@ module NotificationRecipientService def target raise 'abstract' end - - # rubocop:disable Rails/Delegate def project target.project end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 02769e72229..87173cc79ec 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -124,7 +124,7 @@ module Projects # It's possible that the project was destroyed, but some after_commit # hook failed and caused us to end up here. A destroyed model will be a frozen hash, # which cannot be altered. - project.update_attributes(delete_error: message, pending_delete: false) unless project.destroyed? + project.update(delete_error: message, pending_delete: false) unless project.destroyed? log_error("Deletion failed on #{project.full_path} with the following message: #{message}") end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 348eb0bf8d8..a8aafa9fb4f 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -37,7 +37,7 @@ module Projects return new_project unless new_project.persisted? builds_access_level = @project.project_feature.builds_access_level - new_project.project_feature.update_attributes(builds_access_level: builds_access_level) + new_project.project_feature.update(builds_access_level: builds_access_level) link_fork_network(new_project) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d8250cd8102..f4fbaacc08b 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -22,7 +22,7 @@ module Projects # If the block added errors, don't try to save the project return validation_failed! if project.errors.any? - if project.update_attributes(params.except(:default_branch)) + if project.update(params.except(:default_branch)) if project.previous_changes.include?('path') project.rename_repo else diff --git a/app/services/update_release_service.rb b/app/services/update_release_service.rb index b7c36651968..dc696e9c440 100644 --- a/app/services/update_release_service.rb +++ b/app/services/update_release_service.rb @@ -7,7 +7,7 @@ class UpdateReleaseService < BaseService release = project.releases.find_by(tag: tag_name) if release - release.update_attributes(description: release_description) + release.update(description: release_description) success(release) else diff --git a/app/workers/object_storage/migrate_uploads_worker.rb b/app/workers/object_storage/migrate_uploads_worker.rb index a3ecfa8e711..01d03ec7888 100644 --- a/app/workers/object_storage/migrate_uploads_worker.rb +++ b/app/workers/object_storage/migrate_uploads_worker.rb @@ -1,6 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/LineLength -# rubocop:disable Style/Documentation module ObjectStorage class MigrateUploadsWorker diff --git a/config/application.rb b/config/application.rb index 97bc86b3e3a..0304f466734 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,4 +1,4 @@ -require File.expand_path('../boot', __FILE__) +require File.expand_path('boot', __dir__) require 'rails/all' @@ -211,7 +211,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) # rubocop:disable GitlabSecurity/PublicSend + send(name, project&.namespace, project, *args) end end end diff --git a/config/environment.rb b/config/environment.rb index 487a4564b47..5d35937f7c6 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -4,7 +4,7 @@ if %w[1 true].include?(ENV["RAILS5"]) require_relative 'application' else - require File.expand_path('../application', __FILE__) + require File.expand_path('application', __dir__) end # Initialize the rails application diff --git a/config/initializers/active_record_locking.rb b/config/initializers/active_record_locking.rb index 0861544c5a4..21ff323927b 100644 --- a/config/initializers/active_record_locking.rb +++ b/config/initializers/active_record_locking.rb @@ -17,7 +17,7 @@ module ActiveRecord lock_col = self.class.locking_column - previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend + previous_lock_value = send(lock_col).to_i # This line is added as a patch previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0 @@ -47,7 +47,7 @@ module ActiveRecord # If something went wrong, revert the version. rescue Exception - send(lock_col + '=', previous_lock_value) # rubocop:disable GitlabSecurity/PublicSend + send(lock_col + '=', previous_lock_value) raise end end diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index b7aadc27e71..6769855b899 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -112,9 +112,9 @@ module API can_push = params[:can_push].nil? ? deploy_keys_project.can_push : params[:can_push] title = params[:title] || deploy_keys_project.deploy_key.title - result = deploy_keys_project.update_attributes(can_push: can_push, - deploy_key_attributes: { id: params[:key_id], - title: title }) + result = deploy_keys_project.update(can_push: can_push, + deploy_key_attributes: { id: params[:key_id], + title: title }) if result present deploy_keys_project, with: Entities::DeployKeysProject diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 68921ae439b..4760a1c08d7 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -80,7 +80,7 @@ module API update_params = declared_params(include_missing: false) - if hook.update_attributes(update_params) + if hook.update(update_params) present hook, with: Entities::ProjectHook else error!("Invalid url given", 422) if hook.errors[:url].present? diff --git a/lib/api/services.rb b/lib/api/services.rb index 794fdab8f2b..553e8dff4b9 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -787,7 +787,7 @@ module API service = user_project.find_or_initialize_service(service_slug.underscore) service_params = declared_params(include_missing: false).merge(active: true) - if service.update_attributes(service_params) + if service.update(service_params) present service, with: Entities::ProjectService else render_api_error!('400 Bad Request', 400) @@ -807,7 +807,7 @@ module API hash.merge!(key => nil) end - unless service.update_attributes(attrs.merge(active: false)) + unless service.update(attrs.merge(active: false)) render_api_error!('400 Bad Request', 400) end end diff --git a/lib/api/users.rb b/lib/api/users.rb index e8df2c5a74a..5aaaf104dff 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -186,7 +186,7 @@ module API identity = user.identities.find_by(provider: identity_attrs[:provider]) if identity - identity.update_attributes(identity_attrs) + identity.update(identity_attrs) else identity = user.identities.build(identity_attrs) identity.save diff --git a/lib/banzai/filter/markdown_engines/common_mark.rb b/lib/banzai/filter/markdown_engines/common_mark.rb index bc9597df894..dbb25280849 100644 --- a/lib/banzai/filter/markdown_engines/common_mark.rb +++ b/lib/banzai/filter/markdown_engines/common_mark.rb @@ -18,7 +18,7 @@ module Banzai PARSE_OPTIONS = [ :FOOTNOTES, # parse footnotes. :STRIKETHROUGH_DOUBLE_TILDE, # parse strikethroughs by double tildes (as redcarpet does). - :VALIDATE_UTF8 # replace illegal sequences with the replacement character U+FFFD. + :VALIDATE_UTF8 # replace illegal sequences with the replacement character U+FFFD. ].freeze # The `:GITHUB_PRE_LANG` option is not used intentionally because diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index e7283b2f9e8..589e8062226 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -48,7 +48,7 @@ module Gitlab gl_user rescue ActiveRecord::RecordInvalid => e log.info "(#{provider}) Error saving user #{auth_hash.uid} (#{auth_hash.email}): #{gl_user.errors.full_messages}" - return self, e.record.errors + [self, e.record.errors] end def gl_user diff --git a/lib/gitlab/background_migration/add_merge_request_diff_commits_count.rb b/lib/gitlab/background_migration/add_merge_request_diff_commits_count.rb index d5cf9e0d53a..cb2bdea755c 100644 --- a/lib/gitlab/background_migration/add_merge_request_diff_commits_count.rb +++ b/lib/gitlab/background_migration/add_merge_request_diff_commits_count.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true # rubocop:disable Style/Documentation -# rubocop:disable Metrics/LineLength module Gitlab module BackgroundMigration diff --git a/lib/gitlab/background_migration/archive_legacy_traces.rb b/lib/gitlab/background_migration/archive_legacy_traces.rb index 5a4e5b2c471..92096e29ef1 100644 --- a/lib/gitlab/background_migration/archive_legacy_traces.rb +++ b/lib/gitlab/background_migration/archive_legacy_traces.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/AbcSize # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/create_fork_network_memberships_range.rb b/lib/gitlab/background_migration/create_fork_network_memberships_range.rb index 1b4a9e8a194..ccd1f9b4dba 100644 --- a/lib/gitlab/background_migration/create_fork_network_memberships_range.rb +++ b/lib/gitlab/background_migration/create_fork_network_memberships_range.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/LineLength # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb b/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb index c2bf42f846d..da8265a3a5f 100644 --- a/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb +++ b/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/LineLength # rubocop:disable Style/Documentation class Gitlab::BackgroundMigration::CreateGpgKeySubkeysFromGpgKeys diff --git a/lib/gitlab/background_migration/delete_diff_files.rb b/lib/gitlab/background_migration/delete_diff_files.rb index 0b785e1b056..8fb2c334048 100644 --- a/lib/gitlab/background_migration/delete_diff_files.rb +++ b/lib/gitlab/background_migration/delete_diff_files.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/AbcSize # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb index a357538a885..58df74cfa9b 100644 --- a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb +++ b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true # rubocop:disable Metrics/MethodLength -# rubocop:disable Metrics/LineLength # rubocop:disable Metrics/AbcSize # rubocop:disable Style/Documentation diff --git a/lib/gitlab/background_migration/fill_file_store_job_artifact.rb b/lib/gitlab/background_migration/fill_file_store_job_artifact.rb index 22b0ac71920..103bd98af14 100644 --- a/lib/gitlab/background_migration/fill_file_store_job_artifact.rb +++ b/lib/gitlab/background_migration/fill_file_store_job_artifact.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/AbcSize # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/fill_file_store_lfs_object.rb b/lib/gitlab/background_migration/fill_file_store_lfs_object.rb index d0816ae3ed5..77c1f1ffaf0 100644 --- a/lib/gitlab/background_migration/fill_file_store_lfs_object.rb +++ b/lib/gitlab/background_migration/fill_file_store_lfs_object.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/AbcSize # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/fill_store_upload.rb b/lib/gitlab/background_migration/fill_store_upload.rb index 94c65459a67..cba3e21cea6 100644 --- a/lib/gitlab/background_migration/fill_store_upload.rb +++ b/lib/gitlab/background_migration/fill_store_upload.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/AbcSize # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index 242e3143e71..268c6083d3c 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/AbcSize # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb b/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb index 7088aa0860a..38fecac1bfe 100644 --- a/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb +++ b/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/LineLength # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/migrate_system_uploads_to_new_folder.rb b/lib/gitlab/background_migration/migrate_system_uploads_to_new_folder.rb index 7f243073fd0..ef50fe4adb1 100644 --- a/lib/gitlab/background_migration/migrate_system_uploads_to_new_folder.rb +++ b/lib/gitlab/background_migration/migrate_system_uploads_to_new_folder.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/LineLength # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/move_personal_snippet_files.rb b/lib/gitlab/background_migration/move_personal_snippet_files.rb index a4ef51fd0e8..5b2b2af718a 100644 --- a/lib/gitlab/background_migration/move_personal_snippet_files.rb +++ b/lib/gitlab/background_migration/move_personal_snippet_files.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/LineLength # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb b/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb index d9d3d2e667b..698f5e46c0c 100644 --- a/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb +++ b/lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true # rubocop:disable Metrics/MethodLength -# rubocop:disable Metrics/LineLength # rubocop:disable Metrics/ClassLength # rubocop:disable Metrics/BlockLength # rubocop:disable Style/Documentation diff --git a/lib/gitlab/background_migration/populate_fork_networks_range.rb b/lib/gitlab/background_migration/populate_fork_networks_range.rb index a976cb4c243..aa4f130538c 100644 --- a/lib/gitlab/background_migration/populate_fork_networks_range.rb +++ b/lib/gitlab/background_migration/populate_fork_networks_range.rb @@ -19,7 +19,7 @@ module Gitlab create_fork_networks_for_missing_projects(start_id, end_id) create_fork_networks_memberships_for_root_projects(start_id, end_id) - delay = BackgroundMigration::CreateForkNetworkMembershipsRange::RESCHEDULE_DELAY # rubocop:disable Metrics/LineLength + delay = BackgroundMigration::CreateForkNetworkMembershipsRange::RESCHEDULE_DELAY BackgroundMigrationWorker.perform_in( delay, "CreateForkNetworkMembershipsRange", [start_id, end_id] ) diff --git a/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb b/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb index 8a901a9bf39..d89ce358bb9 100644 --- a/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb +++ b/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb @@ -1,7 +1,4 @@ # frozen_string_literal: true -# rubocop:disable Metrics/LineLength -# rubocop:disable Metrics/MethodLength -# rubocop:disable Metrics/ClassLength # rubocop:disable Style/Documentation module Gitlab diff --git a/lib/gitlab/background_migration/populate_untracked_uploads.rb b/lib/gitlab/background_migration/populate_untracked_uploads.rb index 9232f20a063..a19dc9747fb 100644 --- a/lib/gitlab/background_migration/populate_untracked_uploads.rb +++ b/lib/gitlab/background_migration/populate_untracked_uploads.rb @@ -4,7 +4,7 @@ module Gitlab module BackgroundMigration # This class processes a batch of rows in `untracked_files_for_uploads` by # adding each file to the `uploads` table if it does not exist. - class PopulateUntrackedUploads # rubocop:disable Metrics/ClassLength + class PopulateUntrackedUploads def perform(start_id, end_id) return unless migrate? diff --git a/lib/gitlab/background_migration/populate_untracked_uploads_dependencies.rb b/lib/gitlab/background_migration/populate_untracked_uploads_dependencies.rb index a2c5acbde71..4a9a62aaeb5 100644 --- a/lib/gitlab/background_migration/populate_untracked_uploads_dependencies.rb +++ b/lib/gitlab/background_migration/populate_untracked_uploads_dependencies.rb @@ -4,7 +4,7 @@ module Gitlab module PopulateUntrackedUploadsDependencies # This class is responsible for producing the attributes necessary to # track an uploaded file in the `uploads` table. - class UntrackedFile < ActiveRecord::Base # rubocop:disable Metrics/ClassLength, Metrics/LineLength + class UntrackedFile < ActiveRecord::Base # rubocop:disable Metrics/ClassLength self.table_name = 'untracked_files_for_uploads' # Ends with /:random_hex/:filename @@ -134,7 +134,7 @@ module Gitlab # Not including a leading slash def path_relative_to_upload_dir - upload_dir = Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR # rubocop:disable Metrics/LineLength + upload_dir = Gitlab::BackgroundMigration::PrepareUntrackedUploads::RELATIVE_UPLOAD_DIR base = %r{\A#{Regexp.escape(upload_dir)}/} @path_relative_to_upload_dir ||= path.sub(base, '') end diff --git a/lib/gitlab/background_migration/prepare_untracked_uploads.rb b/lib/gitlab/background_migration/prepare_untracked_uploads.rb index 522c69a0bb1..81ca2b0a9b7 100644 --- a/lib/gitlab/background_migration/prepare_untracked_uploads.rb +++ b/lib/gitlab/background_migration/prepare_untracked_uploads.rb @@ -144,7 +144,7 @@ module Gitlab def table_columns_and_values_for_insert(file_paths) values = file_paths.map do |file_path| - ActiveRecord::Base.send(:sanitize_sql_array, ['(?)', file_path]) # rubocop:disable GitlabSecurity/PublicSend, Metrics/LineLength + ActiveRecord::Base.send(:sanitize_sql_array, ['(?)', file_path]) # rubocop:disable GitlabSecurity/PublicSend end.join(', ') "#{UntrackedFile.table_name} (path) VALUES #{values}" diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb index db47c2f6185..7cddd2c7b7e 100644 --- a/lib/gitlab/ci/config/entry/configurable.rb +++ b/lib/gitlab/ci/config/entry/configurable.rb @@ -47,7 +47,7 @@ module Gitlab Hash[(@nodes || {}).map { |key, factory| [key, factory.dup] }] end - private # rubocop:disable Lint/UselessAccessModifier + private def entry(key, entry, metadata) factory = Entry::Factory.new(entry) diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 3cf35f499cd..9147ef401da 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -60,7 +60,7 @@ module Gitlab def in_memory_application_settings with_fallback_to_fake_application_settings do - @in_memory_application_settings ||= ::ApplicationSetting.build_from_defaults # rubocop:disable Gitlab/ModuleWithInstanceVariables + @in_memory_application_settings ||= ::ApplicationSetting.build_from_defaults end end diff --git a/lib/gitlab/encoding_helper.rb b/lib/gitlab/encoding_helper.rb index 0b8f6cfe3cb..5b5071ffc39 100644 --- a/lib/gitlab/encoding_helper.rb +++ b/lib/gitlab/encoding_helper.rb @@ -65,7 +65,7 @@ module Gitlab clean(message) end rescue ArgumentError - return nil + nil end def encode_binary(s) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 420790f45d0..324eac038e1 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1068,8 +1068,6 @@ module Gitlab true end - - # rubocop:disable Metrics/ParameterLists def multi_action( user, branch_name:, message:, actions:, author_email: nil, author_name: nil, diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 6d2278d0876..2716834f566 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -39,7 +39,7 @@ module Gitlab def update_signature!(cached_signature) using_keychain do |gpg_key| - cached_signature.update_attributes!(attributes(gpg_key)) + cached_signature.update!(attributes(gpg_key)) end @signature = cached_signature diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index 344784c866f..db04356a5e9 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -53,7 +53,7 @@ module Gitlab end def config_file - ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || File.expand_path('../../../config/gitlab.yml', __FILE__) + ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || File.expand_path('../../config/gitlab.yml', __dir__) end end end diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index 66f30e3b397..93cd6580d9a 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -180,7 +180,6 @@ module Gitlab @pool end end - # rubocop:enable Gitlab/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index b11520a79bb..f3290e3149c 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -1,5 +1,3 @@ -# rubocop:disable Style/ClassVars - module Gitlab module Metrics # Class for tracking timing information about method calls diff --git a/lib/tasks/gitlab/uploads/migrate.rake b/lib/tasks/gitlab/uploads/migrate.rake index 78e18992a8e..f548a266b99 100644 --- a/lib/tasks/gitlab/uploads/migrate.rake +++ b/lib/tasks/gitlab/uploads/migrate.rake @@ -8,7 +8,7 @@ namespace :gitlab do @uploader_class = args.uploader_class.constantize @model_class = args.model_class.constantize - uploads.each_batch(of: batch_size, &method(:enqueue_batch)) # rubocop: disable Cop/InBatches + uploads.each_batch(of: batch_size, &method(:enqueue_batch)) end def enqueue_batch(batch, index) diff --git a/qa/qa/runtime/release.rb b/qa/qa/runtime/release.rb index 12e56404cf6..4f83a773645 100644 --- a/qa/qa/runtime/release.rb +++ b/qa/qa/runtime/release.rb @@ -21,7 +21,7 @@ module QA end def self.method_missing(name, *args) - self.new.strategy.public_send(name, *args) # rubocop:disable GitlabSecurity/PublicSend + self.new.strategy.public_send(name, *args) end end end diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb index e20623c0ac1..945b6142abf 100644 --- a/spec/controllers/projects/forks_controller_spec.rb +++ b/spec/controllers/projects/forks_controller_spec.rb @@ -31,7 +31,7 @@ describe Projects::ForksController do context 'when fork is private' do before do - forked_project.update_attributes(visibility_level: Project::PRIVATE, group: group) + forked_project.update(visibility_level: Project::PRIVATE, group: group) end it 'is not be visible for non logged in users' do diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index 72f6af112b3..78c6f7839b4 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -23,7 +23,7 @@ describe Projects::GroupLinksController do context 'when project is not allowed to be shared with a group' do before do - group.update_attributes(share_with_group_lock: false) + group.update(share_with_group_lock: false) end include_context 'link project to group' diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 812833cc86b..6f06210f3de 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -29,7 +29,7 @@ describe Projects::ImportsController do context 'when import is in progress' do before do - project.update_attributes(import_status: :started) + project.update(import_status: :started) end it 'renders template' do @@ -47,7 +47,7 @@ describe Projects::ImportsController do context 'when import failed' do before do - project.update_attributes(import_status: :failed) + project.update(import_status: :failed) end it 'redirects to new_namespace_project_import_path' do @@ -59,7 +59,7 @@ describe Projects::ImportsController do context 'when import finished' do before do - project.update_attributes(import_status: :finished) + project.update(import_status: :finished) end context 'when project is a fork' do @@ -108,7 +108,7 @@ describe Projects::ImportsController do context 'when import never happened' do before do - project.update_attributes(import_status: :none) + project.update(import_status: :none) end it 'redirects to namespace_project_path' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 7f5f0b76c51..444415011a9 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -315,7 +315,7 @@ describe Projects::MergeRequestsController do context 'when the merge request is not mergeable' do before do - merge_request.update_attributes(title: "WIP: #{merge_request.title}") + merge_request.update(title: "WIP: #{merge_request.title}") post :merge, base_params end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index a2dfc43e9f7..fd7d867f9e5 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -166,7 +166,7 @@ describe ProjectsController do User.project_views.keys.each do |project_view| context "with #{project_view} view set" do before do - user.update_attributes(project_view: project_view) + user.update(project_view: project_view) get :show, namespace_id: empty_project.namespace, id: empty_project end @@ -188,7 +188,7 @@ describe ProjectsController do User.project_views.keys.each do |project_view| context "with #{project_view} view set" do before do - user.update_attributes(project_view: project_view) + user.update(project_view: project_view) get :show, namespace_id: empty_project.namespace, id: empty_project end diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 3a35bdd25de..4d4f74e101e 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -27,7 +27,7 @@ FactoryBot.define do end after(:create) do |issue, evaluator| - issue.update_attributes(labels: evaluator.labels) + issue.update(labels: evaluator.labels) end end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 3441ce1b8cb..f722bb9cb0d 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -117,7 +117,7 @@ FactoryBot.define do end after(:create) do |merge_request, evaluator| - merge_request.update_attributes(labels: evaluator.labels) + merge_request.update(labels: evaluator.labels) end end end diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 87fa3f60826..29fcf89fa7a 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -89,7 +89,7 @@ describe 'Commits' do context 'Download artifacts' do before do - build.update_attributes(legacy_artifacts_file: artifacts_file) + build.update(legacy_artifacts_file: artifacts_file) end it do @@ -146,7 +146,7 @@ describe 'Commits' do context "when logged as reporter" do before do project.add_reporter(user) - build.update_attributes(legacy_artifacts_file: artifacts_file) + build.update(legacy_artifacts_file: artifacts_file) visit pipeline_path(pipeline) end @@ -168,7 +168,7 @@ describe 'Commits' do project.update( visibility_level: Gitlab::VisibilityLevel::INTERNAL, public_builds: false) - build.update_attributes(legacy_artifacts_file: artifacts_file) + build.update(legacy_artifacts_file: artifacts_file) visit pipeline_path(pipeline) end diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index 8647616101b..4daacc61d85 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -59,7 +59,7 @@ describe 'Dashboard Projects' do context 'when last_repository_updated_at, last_activity_at and update_at are present' do it 'shows the last_repository_updated_at attribute as the update date' do - project.update_attributes!(last_repository_updated_at: Time.now, last_activity_at: 1.hour.ago) + project.update!(last_repository_updated_at: Time.now, last_activity_at: 1.hour.ago) visit dashboard_projects_path @@ -67,7 +67,7 @@ describe 'Dashboard Projects' do end it 'shows the last_activity_at attribute as the update date' do - project.update_attributes!(last_repository_updated_at: 1.hour.ago, last_activity_at: Time.now) + project.update!(last_repository_updated_at: 1.hour.ago, last_activity_at: Time.now) visit dashboard_projects_path @@ -77,7 +77,7 @@ describe 'Dashboard Projects' do context 'when last_repository_updated_at and last_activity_at are missing' do it 'shows the updated_at attribute as the update date' do - project.update_attributes!(last_repository_updated_at: nil, last_activity_at: nil) + project.update!(last_repository_updated_at: nil, last_activity_at: nil) project.touch visit dashboard_projects_path diff --git a/spec/features/groups/members/request_access_spec.rb b/spec/features/groups/members/request_access_spec.rb index 1c833c36a61..94510f917a3 100644 --- a/spec/features/groups/members/request_access_spec.rb +++ b/spec/features/groups/members/request_access_spec.rb @@ -13,7 +13,7 @@ describe 'Groups > Members > Request access' do end it 'request access feature is disabled' do - group.update_attributes(request_access_enabled: false) + group.update(request_access_enabled: false) visit group_path(group) expect(page).not_to have_content 'Request Access' diff --git a/spec/features/issues/issue_sidebar_spec.rb b/spec/features/issues/issue_sidebar_spec.rb index b4cb3835a13..3050f23c130 100644 --- a/spec/features/issues/issue_sidebar_spec.rb +++ b/spec/features/issues/issue_sidebar_spec.rb @@ -112,7 +112,7 @@ describe 'Issue Sidebar' do context 'editing issue labels', :js do before do - issue.update_attributes(labels: [label]) + issue.update(labels: [label]) page.within('.block.labels') do find('.edit-link').click end diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index 13cc5f256eb..02d19db3828 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -198,7 +198,7 @@ describe 'Merge request > User posts diff notes', :js do context 'when the MR only supports legacy diff notes' do before do - merge_request.merge_request_diff.update_attributes(start_commit_sha: nil) + merge_request.merge_request_diff.update(start_commit_sha: nil) visit diffs_project_merge_request_path(project, merge_request, view: 'inline') end diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb index f9c6ff90ca1..5e3569e4beb 100644 --- a/spec/features/profiles/password_spec.rb +++ b/spec/features/profiles/password_spec.rb @@ -117,7 +117,7 @@ describe 'Profile > Password' do before do sign_in(user) - user.update_attributes(password_expires_at: 1.hour.ago) + user.update(password_expires_at: 1.hour.ago) user.identities.delete expect(user.ldap_user?).to eq false end diff --git a/spec/features/projects/files/user_browses_files_spec.rb b/spec/features/projects/files/user_browses_files_spec.rb index 41f6c52fb8a..9b1395d5e0a 100644 --- a/spec/features/projects/files/user_browses_files_spec.rb +++ b/spec/features/projects/files/user_browses_files_spec.rb @@ -147,8 +147,6 @@ describe "User browses files" do page.within(".tree-table") do click_link("README.md") end - - # rubocop:disable Lint/Void # Test the full URLs of links instead of relative paths by `have_link(text: "...", href: "...")`. find("a", text: /^empty$/)["href"] == project_blob_url(project, "markdown/d/README.md") # rubocop:enable Lint/Void diff --git a/spec/features/projects/jobs/permissions_spec.rb b/spec/features/projects/jobs/permissions_spec.rb index e9588daf37d..e639f0cf82e 100644 --- a/spec/features/projects/jobs/permissions_spec.rb +++ b/spec/features/projects/jobs/permissions_spec.rb @@ -90,7 +90,7 @@ describe 'Project Jobs Permissions' do before do archive = fixture_file_upload('spec/fixtures/ci_build_artifacts.zip') - job.update_attributes(legacy_artifacts_file: archive) + job.update(legacy_artifacts_file: archive) end context 'when public access for jobs is disabled' do diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index c742eb79392..35b1c46ecf6 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -187,7 +187,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do context "Download artifacts" do before do - job.update_attributes(legacy_artifacts_file: artifacts_file) + job.update(legacy_artifacts_file: artifacts_file) visit project_job_path(project, job) end @@ -198,8 +198,8 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do context 'Artifacts expire date' do before do - job.update_attributes(legacy_artifacts_file: artifacts_file, - artifacts_expire_at: expire_at) + job.update(legacy_artifacts_file: artifacts_file, + artifacts_expire_at: expire_at) visit project_job_path(project, job) end @@ -530,14 +530,14 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do describe "GET /:project/jobs/:id/download" do before do - job.update_attributes(legacy_artifacts_file: artifacts_file) + job.update(legacy_artifacts_file: artifacts_file) visit project_job_path(project, job) click_link 'Download' end context "Build from other project" do before do - job2.update_attributes(legacy_artifacts_file: artifacts_file) + job2.update(legacy_artifacts_file: artifacts_file) visit download_project_job_artifacts_path(project, job2) end diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb index 35d6ac1c650..5599cc9bf1b 100644 --- a/spec/features/projects/members/user_requests_access_spec.rb +++ b/spec/features/projects/members/user_requests_access_spec.rb @@ -11,7 +11,7 @@ describe 'Projects > Members > User requests access', :js do end it 'request access feature is disabled' do - project.update_attributes(request_access_enabled: false) + project.update(request_access_enabled: false) visit project_path(project) expect(page).not_to have_content 'Request Access' diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index 9c165b17704..7d47e342e92 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -606,7 +606,7 @@ describe 'Pipelines', :js do describe 'user clicks the button' do context 'when project already has jobs_cache_index' do before do - project.update_attributes(jobs_cache_index: 1) + project.update(jobs_cache_index: 1) end it 'increments jobs_cache_index' do diff --git a/spec/features/projects/remote_mirror_spec.rb b/spec/features/projects/remote_mirror_spec.rb index 53a3f6f474b..97db4a2b8f2 100644 --- a/spec/features/projects/remote_mirror_spec.rb +++ b/spec/features/projects/remote_mirror_spec.rb @@ -13,7 +13,7 @@ describe 'Project remote mirror', :feature do context 'when last_error is present but last_update_at is not' do it 'renders error message without timstamp' do - remote_mirror.update_attributes(last_error: 'Some new error', last_update_at: nil) + remote_mirror.update(last_error: 'Some new error', last_update_at: nil) visit project_mirror_path(project) @@ -23,7 +23,7 @@ describe 'Project remote mirror', :feature do context 'when last_error and last_update_at are present' do it 'renders error message with timestamp' do - remote_mirror.update_attributes(last_error: 'Some new error', last_update_at: Time.now - 5.minutes) + remote_mirror.update(last_error: 'Some new error', last_update_at: Time.now - 5.minutes) visit project_mirror_path(project) diff --git a/spec/features/projects/services/user_activates_slack_notifications_spec.rb b/spec/features/projects/services/user_activates_slack_notifications_spec.rb index fae9ebd1bd6..727b1fe2d11 100644 --- a/spec/features/projects/services/user_activates_slack_notifications_spec.rb +++ b/spec/features/projects/services/user_activates_slack_notifications_spec.rb @@ -29,7 +29,7 @@ describe 'User activates Slack notifications' do context 'when service is already configured' do before do service.fields - service.update_attributes( + service.update( push_channel: 1, issue_channel: 2, merge_request_channel: 3, diff --git a/spec/features/projects/show/user_sees_deletion_failure_message_spec.rb b/spec/features/projects/show/user_sees_deletion_failure_message_spec.rb index aa23bef6fd8..d9d57298929 100644 --- a/spec/features/projects/show/user_sees_deletion_failure_message_spec.rb +++ b/spec/features/projects/show/user_sees_deletion_failure_message_spec.rb @@ -8,7 +8,7 @@ describe 'Projects > Show > User sees a deletion failure message' do end it 'shows error message if deletion for project fails' do - project.update_attributes(delete_error: "Something went wrong", pending_delete: false) + project.update(delete_error: "Something went wrong", pending_delete: false) visit project_path(project) diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb index db141ef7096..bc9443c6093 100644 --- a/spec/features/signed_commits_spec.rb +++ b/spec/features/signed_commits_spec.rb @@ -23,7 +23,7 @@ describe 'GPG signed commits', :js do # user changes his email which makes the gpg key verified Sidekiq::Testing.inline! do user.skip_reconfirmation! - user.update_attributes!(email: GpgHelpers::User1.emails.first) + user.update!(email: GpgHelpers::User1.emails.first) end visit project_commits_path(project, :'signed-commits') diff --git a/spec/finders/group_projects_finder_spec.rb b/spec/finders/group_projects_finder_spec.rb index be80ee7d767..0a69c03e491 100644 --- a/spec/finders/group_projects_finder_spec.rb +++ b/spec/finders/group_projects_finder_spec.rb @@ -81,7 +81,7 @@ describe GroupProjectsFinder do context "with external user" do before do - current_user.update_attributes(external: true) + current_user.update(external: true) end it { is_expected.to match_array([shared_project_2, shared_project_1]) } @@ -112,7 +112,7 @@ describe GroupProjectsFinder do context "with external user" do before do - current_user.update_attributes(external: true) + current_user.update(external: true) end context 'with subgroups projects', :nested_groups do diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb index 29a47e005a6..7f77a713b12 100644 --- a/spec/finders/joined_groups_finder_spec.rb +++ b/spec/finders/joined_groups_finder_spec.rb @@ -53,7 +53,7 @@ describe JoinedGroupsFinder do context 'external users' do before do - profile_visitor.update_attributes(external: true) + profile_visitor.update(external: true) end context 'if not a member' do diff --git a/spec/finders/move_to_project_finder_spec.rb b/spec/finders/move_to_project_finder_spec.rb index 74639d4147f..e1faf3d569c 100644 --- a/spec/finders/move_to_project_finder_spec.rb +++ b/spec/finders/move_to_project_finder_spec.rb @@ -45,7 +45,7 @@ describe MoveToProjectFinder do it 'does not return projects for which issues are disabled' do reporter_project.add_reporter(user) - reporter_project.update_attributes(issues_enabled: false) + reporter_project.update(issues_enabled: false) other_reporter_project = create(:project) other_reporter_project.add_reporter(user) diff --git a/spec/finders/personal_projects_finder_spec.rb b/spec/finders/personal_projects_finder_spec.rb index 00c551a1f65..ef7dd0cd4a8 100644 --- a/spec/finders/personal_projects_finder_spec.rb +++ b/spec/finders/personal_projects_finder_spec.rb @@ -35,7 +35,7 @@ describe PersonalProjectsFinder do context 'external' do before do - current_user.update_attributes(external: true) + current_user.update(external: true) end it { is_expected.to eq([public_project, private_project]) } diff --git a/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb b/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb index ee60e498b59..2e77e80ee46 100644 --- a/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb +++ b/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::BackgroundMigration::MovePersonalSnippetFiles do let(:snippet) do snippet = create(:personal_snippet) create_upload_for_snippet(snippet) - snippet.update_attributes!(description: markdown_linking_file(snippet)) + snippet.update!(description: markdown_linking_file(snippet)) snippet end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb index b411aaa19da..0a8c77b0ad9 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb @@ -281,7 +281,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, : it "doesn't break when the namespace was renamed" do subject.rename_namespace(namespace) - namespace.update_attributes!(path: 'renamed-afterwards') + namespace.update!(path: 'renamed-afterwards') expect { subject.revert_renames }.not_to raise_error end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb index b4896d69077..d4d7a83921c 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb @@ -169,7 +169,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de it "doesn't break when the project was renamed" do subject.rename_project(project) - project.update_attributes!(path: 'renamed-afterwards') + project.update!(path: 'renamed-afterwards') expect { subject.revert_renames }.not_to raise_error end diff --git a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb index 6fbffc38444..1a2c6ef25c4 100644 --- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb +++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb @@ -141,7 +141,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do expect(invalid_gpg_signature.reload.verification_status).to eq 'unverified_key' # InvalidGpgSignatureUpdater is called by the after_update hook - user.update_attributes!(email: GpgHelpers::User1.emails.first) + user.update!(email: GpgHelpers::User1.emails.first) expect(invalid_gpg_signature.reload).to have_attributes( project: project, @@ -166,7 +166,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do ) # InvalidGpgSignatureUpdater is called by the after_update hook - user.update_attributes!(email: 'still.unrelated@example.com') + user.update!(email: 'still.unrelated@example.com') expect(invalid_gpg_signature.reload).to have_attributes( project: project, diff --git a/spec/migrations/issues_moved_to_id_foreign_key_spec.rb b/spec/migrations/issues_moved_to_id_foreign_key_spec.rb index dd2b08099f2..495e86ee888 100644 --- a/spec/migrations/issues_moved_to_id_foreign_key_spec.rb +++ b/spec/migrations/issues_moved_to_id_foreign_key_spec.rb @@ -14,7 +14,7 @@ describe IssuesMovedToIdForeignKey, :migration, schema: 20171114150259 do it 'removes the orphaned moved_to_id' do subject.down - issue_third.update_attributes(moved_to_id: 100000) + issue_third.update(moved_to_id: 100000) subject.up diff --git a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb index df009cec25c..6f729fec460 100644 --- a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb @@ -12,7 +12,7 @@ describe MigrateGcpClustersToNewClustersArchitectures, :migration do class KubernetesService < ActiveRecord::Base self.table_name = 'services' - serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize + serialize :properties, JSON default_value_for :active, true default_value_for :type, 'KubernetesService' diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 7e75d5a5411..6dba132184c 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -30,7 +30,7 @@ describe Ci::BuildMetadata do context 'when runner is assigned to the job' do before do - build.update_attributes(runner: runner) + build.update(runner: runner) end context 'when runner timeout is lower than project timeout' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 090ca159e08..234d2d8aa3a 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -186,18 +186,18 @@ describe Ci::Build do let(:runner) { create(:ci_runner, :project, projects: [build.project]) } before do - runner.update_attributes(contacted_at: 1.second.ago) + runner.update(contacted_at: 1.second.ago) end it { is_expected.to be_truthy } it 'that is inactive' do - runner.update_attributes(active: false) + runner.update(active: false) is_expected.to be_falsey end it 'that is not online' do - runner.update_attributes(contacted_at: nil) + runner.update(contacted_at: nil) is_expected.to be_falsey end @@ -261,7 +261,7 @@ describe Ci::Build do context 'artifacts metadata does not exist' do before do - build.update_attributes(legacy_artifacts_metadata: nil) + build.update(legacy_artifacts_metadata: nil) end it { is_expected.to be_falsy } @@ -1535,7 +1535,7 @@ describe Ci::Build do expect(ProjectStatistics) .not_to receive(:increment_statistic) - build.project.update_attributes(pending_delete: true) + build.project.update(pending_delete: true) build.project.destroy! end end @@ -1662,7 +1662,7 @@ describe Ci::Build do end before do - build.update_attributes(user: user) + build.update(user: user) end it { user_variables.each { |v| is_expected.to include(v) } } @@ -1740,7 +1740,7 @@ describe Ci::Build do context 'when build started manually' do before do - build.update_attributes(when: :manual) + build.update(when: :manual) end let(:manual_variable) do @@ -1756,7 +1756,7 @@ describe Ci::Build do end before do - build.update_attributes(tag: true) + build.update(tag: true) end it { is_expected.to include(tag_variable) } diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 2f87fb5f25d..0fd7612c011 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -163,7 +163,7 @@ describe Ci::JobArtifact do expect(ProjectStatistics) .not_to receive(:increment_statistic) - project.update_attributes(pending_delete: true) + project.update(pending_delete: true) project.destroy! end end diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb index c16b245bea8..e5392fe0462 100644 --- a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb +++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb @@ -4,8 +4,8 @@ describe BatchDestroyDependentAssociations do class TestProject < ActiveRecord::Base self.table_name = 'projects' - has_many :builds, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :builds, dependent: :destroy + has_many :notification_settings, as: :source, dependent: :delete_all has_many :pages_domains has_many :todos diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index c73ea6aa94c..a9b237fa9ea 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -136,7 +136,7 @@ describe Issue, "Mentionable" do expect(SystemNoteService).not_to receive(:cross_reference) - issue.update_attributes(description: 'New description') + issue.update(description: 'New description') issue.create_new_cross_references! end @@ -145,7 +145,7 @@ describe Issue, "Mentionable" do expect(SystemNoteService).to receive(:cross_reference).with(issues[1], any_args) - issue.update_attributes(description: issues[1].to_reference) + issue.update(description: issues[1].to_reference) issue.create_new_cross_references! end @@ -155,7 +155,7 @@ describe Issue, "Mentionable" do expect(SystemNoteService).to receive(:cross_reference).with(issues[1], any_args) - note.update_attributes(note: issues[1].to_reference) + note.update(note: issues[1].to_reference) note.create_new_cross_references! end end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 8cb50d7465c..ed3e28fbeca 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -29,7 +29,7 @@ describe Group, 'Routable' do end it 'updates route record on path change' do - group.update_attributes(path: 'wow', name: 'much') + group.update(path: 'wow', name: 'much') expect(group.route.path).to eq('wow') expect(group.route.name).to eq('much') diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 25d6597084c..4bded9efe91 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -562,7 +562,7 @@ describe Environment do it "is not regenerated if name changes" do original_slug = environment.slug - environment.update_attributes!(name: environment.name.reverse) + environment.update!(name: environment.name.reverse) expect(environment.slug).to eq(original_slug) end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 9fe1186a8c9..aeec485358e 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -617,7 +617,7 @@ describe Group do expect(group).to receive(:system_hook_service).and_return(system_hook_service) expect(system_hook_service).to receive(:execute_hooks_for).with(group, :rename) - group.update_attributes!(path: new_path) + group.update!(path: new_path) end end @@ -625,7 +625,7 @@ describe Group do it 'does not trigger system hook' do expect(group).not_to receive(:system_hook_service) - group.update_attributes!(name: 'new name') + group.update!(name: 'new name') end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8c6b411ec9a..c7eead2bdaa 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1891,7 +1891,7 @@ describe MergeRequest do end it 'returns false if the merge request is merged' do - merge_request.update_attributes(state: 'merged') + merge_request.update(state: 'merged') expect(merge_request.reload.reopenable?).to be_falsey end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 70f1a1c8b38..c1b385aaf76 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -200,7 +200,7 @@ describe Namespace do end it "moves dir if path changed" do - namespace.update_attributes(path: namespace.full_path + '_new') + namespace.update(path: namespace.full_path + '_new') expect(gitlab_shell.exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy end @@ -279,7 +279,7 @@ describe Namespace do it "repository directory remains unchanged if path changed" do before_disk_path = project.disk_path - namespace.update_attributes(path: namespace.full_path + '_new') + namespace.update(path: namespace.full_path + '_new') expect(before_disk_path).to eq(project.disk_path) expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 12681a147b4..d7c5f26ab67 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -58,7 +58,7 @@ RSpec.describe NotificationSetting do 1.upto(4) do |i| setting = create(:notification_setting, user: user) - setting.project.update_attributes(pending_delete: true) if i.even? + setting.project.update(pending_delete: true) if i.even? end end diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 63c6fbda3f2..cd7f77024da 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -77,7 +77,7 @@ describe ProjectFeature do context 'repository related features' do before do - project.project_feature.update_attributes( + project.project_feature.update( merge_requests_access_level: ProjectFeature::DISABLED, builds_access_level: ProjectFeature::DISABLED, repository_access_level: ProjectFeature::PRIVATE diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b9512b81678..bbf37ca59b9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -567,15 +567,15 @@ describe Project do end it 'returns the most recent timestamp' do - project.update_attributes(updated_at: nil, - last_activity_at: timestamp, - last_repository_updated_at: timestamp - 1.hour) + project.update(updated_at: nil, + last_activity_at: timestamp, + last_repository_updated_at: timestamp - 1.hour) expect(project.last_activity_date).to be_like_time(timestamp) - project.update_attributes(updated_at: timestamp, - last_activity_at: timestamp - 1.hour, - last_repository_updated_at: nil) + project.update(updated_at: timestamp, + last_activity_at: timestamp - 1.hour, + last_repository_updated_at: nil) expect(project.last_activity_date).to be_like_time(timestamp) end @@ -1768,7 +1768,7 @@ describe Project do it 'resets project import_error' do error_message = 'Some error' mirror = create(:project_empty_repo, :import_started) - mirror.import_state.update_attributes(last_error: error_message) + mirror.import_state.update(last_error: error_message) expect { mirror.import_finish }.to change { mirror.import_error }.from(error_message).to(nil) end @@ -1929,7 +1929,7 @@ describe Project do end it 'returns false when remote mirror is disabled' do - project.remote_mirrors.first.update_attributes(enabled: false) + project.remote_mirrors.first.update(enabled: false) is_expected.to be_falsy end @@ -1959,7 +1959,7 @@ describe Project do end it 'does not sync disabled remote mirrors' do - project.remote_mirrors.first.update_attributes(enabled: false) + project.remote_mirrors.first.update(enabled: false) expect_any_instance_of(RemoteMirror).not_to receive(:sync) diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 3597b080021..c2ef0435c8e 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -85,7 +85,7 @@ describe RemoteMirror do expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original - mirror.update_attributes(url: 'http://test.com') + mirror.update(url: 'http://test.com') end end end @@ -167,7 +167,7 @@ describe RemoteMirror do context 'with remote mirroring disabled' do it 'returns nil' do - remote_mirror.update_attributes(enabled: false) + remote_mirror.update(enabled: false) expect(remote_mirror.sync).to be_nil end @@ -229,7 +229,7 @@ describe RemoteMirror do end before do - remote_mirror.update_attributes(last_update_started_at: Time.now) + remote_mirror.update(last_update_started_at: Time.now) end context 'when remote mirror does not have status failed' do @@ -244,7 +244,7 @@ describe RemoteMirror do context 'when remote mirror has status failed' do it 'returns false when last update started after the timestamp' do - remote_mirror.update_attributes(update_status: 'failed') + remote_mirror.update(update_status: 'failed') expect(remote_mirror.updated_since?(timestamp)).to be false end diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index 01238a89a81..48799781b87 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -29,12 +29,12 @@ describe Route do context 'after update' do it 'calls #create_redirect_for_old_path' do expect(route).to receive(:create_redirect_for_old_path) - route.update_attributes(path: 'foo') + route.update(path: 'foo') end it 'calls #delete_conflicting_redirects' do expect(route).to receive(:delete_conflicting_redirects) - route.update_attributes(path: 'foo') + route.update(path: 'foo') end end @@ -70,7 +70,7 @@ describe Route do context 'path update' do context 'when route name is set' do before do - route.update_attributes(path: 'bar') + route.update(path: 'bar') end it 'updates children routes with new path' do @@ -89,7 +89,7 @@ describe Route do end it "does not fail" do - expect(route.update_attributes(path: 'bar')).to be_truthy + expect(route.update(path: 'bar')).to be_truthy end end @@ -100,7 +100,7 @@ describe Route do let!(:conflicting_redirect3) { route.create_redirect('gitlab-org') } it 'deletes the conflicting redirects' do - route.update_attributes(path: 'bar') + route.update(path: 'bar') expect(RedirectRoute.exists?(path: 'bar/test')).to be_falsey expect(RedirectRoute.exists?(path: 'bar/test/foo')).to be_falsey @@ -111,7 +111,7 @@ describe Route do context 'name update' do it 'updates children routes with new path' do - route.update_attributes(name: 'bar') + route.update(name: 'bar') expect(described_class.exists?(name: 'bar')).to be_truthy expect(described_class.exists?(name: 'bar / test')).to be_truthy @@ -123,7 +123,7 @@ describe Route do # Note: using `update_columns` to skip all validation and callbacks route.update_columns(name: nil) - expect { route.update_attributes(name: 'bar') } + expect { route.update(name: 'bar') } .to change { route.name }.from(nil).to('bar') end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index a849af062c5..029ad7f3e9f 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -280,7 +280,7 @@ describe Service do service.save! expect do - service.update_attributes(active: false) + service.update(active: false) end.to change { service.project.has_external_issue_tracker }.from(true).to(false) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 097144d04ce..164c27d2416 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -383,7 +383,7 @@ describe User do let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } it 'allows a verfied secondary email to be used as the primary without needing reconfirmation' do - user.update_attributes!(email: secondary.email) + user.update!(email: secondary.email) user.reload expect(user.email).to eq secondary.email expect(user.unconfirmed_email).to eq nil @@ -405,11 +405,11 @@ describe User do it 'gets called when email updated' do expect(@user).to receive(:update_emails_with_primary_email) - @user.update_attributes!(email: 'new_primary@example.com') + @user.update!(email: 'new_primary@example.com') end it 'adds old primary to secondary emails when secondary is a new email ' do - @user.update_attributes!(email: 'new_primary@example.com') + @user.update!(email: 'new_primary@example.com') @user.reload expect(@user.emails.count).to eq 2 @@ -417,7 +417,7 @@ describe User do end it 'adds old primary to secondary emails if secondary is becoming a primary' do - @user.update_attributes!(email: @secondary.email) + @user.update!(email: @secondary.email) @user.reload expect(@user.emails.count).to eq 1 @@ -425,7 +425,7 @@ describe User do end it 'transfers old confirmation values into new secondary' do - @user.update_attributes!(email: @secondary.email) + @user.update!(email: @secondary.email) @user.reload expect(@user.emails.count).to eq 1 @@ -494,12 +494,12 @@ describe User do it 'does nothing when the name is updated' do expect(user).not_to receive(:update_invalid_gpg_signatures) - user.update_attributes!(name: 'Bette') + user.update!(name: 'Bette') end it 'synchronizes the gpg keys when the email is updated' do expect(user).to receive(:update_invalid_gpg_signatures).at_most(:twice) - user.update_attributes!(email: 'shawnee.ritchie@denesik.com') + user.update!(email: 'shawnee.ritchie@denesik.com') end end end @@ -617,13 +617,13 @@ describe User do it 'receives callback when external changes' do expect(user).to receive(:ensure_user_rights_and_limits) - user.update_attributes(external: false) + user.update(external: false) end it 'ensures correct rights and limits for user' do stub_config_setting(default_can_create_group: true) - expect { user.update_attributes(external: false) }.to change { user.can_create_group }.to(true) + expect { user.update(external: false) }.to change { user.can_create_group }.to(true) .and change { user.projects_limit }.to(Gitlab::CurrentSettings.default_projects_limit) end end @@ -634,11 +634,11 @@ describe User do it 'receives callback when external changes' do expect(user).to receive(:ensure_user_rights_and_limits) - user.update_attributes(external: true) + user.update(external: true) end it 'ensures correct rights and limits for user' do - expect { user.update_attributes(external: true) }.to change { user.can_create_group }.to(false) + expect { user.update(external: true) }.to change { user.can_create_group }.to(false) .and change { user.projects_limit }.to(0) end end @@ -2461,18 +2461,18 @@ describe User do it 'changes the namespace (just to compare to when username is not changed)' do expect do - user.update_attributes!(username: new_username) + user.update!(username: new_username) end.to change { user.namespace.updated_at } end it 'updates the namespace name' do - user.update_attributes!(username: new_username) + user.update!(username: new_username) expect(user.namespace.name).to eq(new_username) end it 'updates the namespace path' do - user.update_attributes!(username: new_username) + user.update!(username: new_username) expect(user.namespace.path).to eq(new_username) end @@ -2481,12 +2481,12 @@ describe User do let!(:conflicting_namespace) { create(:group, path: new_username) } it 'causes the user save to fail' do - expect(user.update_attributes(username: new_username)).to be_falsey + expect(user.update(username: new_username)).to be_falsey expect(user.namespace.errors.messages[:path].first).to eq('has already been taken') end it 'adds the namespace errors to the user' do - user.update_attributes(username: new_username) + user.update(username: new_username) expect(user.errors.full_messages.first).to eq('Username has already been taken') end @@ -2496,7 +2496,7 @@ describe User do context 'when the username is not changed' do it 'does not change the namespace' do expect do - user.update_attributes!(email: 'asdf@asdf.com') + user.update!(email: 'asdf@asdf.com') end.not_to change { user.namespace.updated_at } end end @@ -2526,7 +2526,7 @@ describe User do expect(system_hook_service).to receive(:execute_hooks_for).with(user, :rename) expect(user).to receive(:system_hook_service).and_return(system_hook_service) - user.update_attributes!(username: new_username) + user.update!(username: new_username) end end @@ -2534,7 +2534,7 @@ describe User do it 'does not trigger system hook' do expect(user).not_to receive(:system_hook_service) - user.update_attributes!(email: 'asdf@asdf.com') + user.update!(email: 'asdf@asdf.com') end end end diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb index 24389f28b21..ffca7ab8e45 100644 --- a/spec/requests/api/access_requests_spec.rb +++ b/spec/requests/api/access_requests_spec.rb @@ -88,7 +88,7 @@ describe API::AccessRequests do context 'when authenticated as a stranger' do context "when access request is disabled for the #{source_type}" do before do - source.update_attributes(request_access_enabled: false) + source.update(request_access_enabled: false) end it 'returns 403' do diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index d8a51f36dba..0a789d58fd8 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -201,7 +201,7 @@ describe API::Helpers do end it 'does not allow expired tokens' do - personal_access_token.update_attributes!(expires_at: 1.day.ago) + personal_access_token.update!(expires_at: 1.day.ago) env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token expect { current_user }.to raise_error Gitlab::Auth::ExpiredError diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index dd568c24c72..f4cf5bd0530 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -71,7 +71,7 @@ describe API::Notes do context "issue is confidential" do before do - ext_issue.update_attributes(confidential: true) + ext_issue.update(confidential: true) end it "returns 404" do @@ -104,7 +104,7 @@ describe API::Notes do context "when issue is confidential" do before do - issue.update_attributes(confidential: true) + issue.update(confidential: true) end it "returns 404" do diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 91d4d5d3de9..173f2a0dea0 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -22,7 +22,7 @@ describe API::PipelineSchedules do .each do |pipeline_schedule| create(:user).tap do |user| project.add_developer(user) - pipeline_schedule.update_attributes(owner: user) + pipeline_schedule.update(owner: user) end pipeline_schedule.pipelines << build(:ci_pipeline, project: project) end diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 97dffdc9233..24b7835abf5 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -157,7 +157,7 @@ describe API::ProjectImport do it 'returns the import status and the error if failed' do project = create(:project, :import_failed) project.add_master(user) - project.import_state.update_attributes(last_error: 'error') + project.import_state.update(last_error: 'error') get api("/projects/#{project.id}/import", user) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index abf9ad738bd..de540ba7a10 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -312,7 +312,7 @@ describe API::Projects do before do project_member - user3.update_attributes(starred_projects: [project, project2, project3, public_project]) + user3.update(starred_projects: [project, project2, project3, public_project]) end it 'returns the starred projects viewable by the user' do @@ -333,7 +333,7 @@ describe API::Projects do let!(:project9) { create(:project, :public, path: 'gitlab9') } before do - user.update_attributes(starred_projects: [project5, project7, project8, project9]) + user.update(starred_projects: [project5, project7, project8, project9]) end context 'including owned filter' do @@ -1451,7 +1451,7 @@ describe API::Projects do end it 'updates visibility_level from public to private' do - project3.update_attributes({ visibility_level: Gitlab::VisibilityLevel::PUBLIC }) + project3.update({ visibility_level: Gitlab::VisibilityLevel::PUBLIC }) project_param = { visibility: 'private' } put api("/projects/#{project3.id}", user), project_param diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index 969710d6613..8df08dd1818 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -109,7 +109,7 @@ describe API::Tags do before do release = project.releases.find_or_initialize_by(tag: tag_name) - release.update_attributes(description: description) + release.update(description: description) end it 'returns an array of project tags with release info' do @@ -400,7 +400,7 @@ describe API::Tags do context 'on tag with existing release' do before do release = project.releases.find_or_initialize_by(tag: tag_name) - release.update_attributes(description: description) + release.update(description: description) end it 'returns 409 if there is already a release' do @@ -422,7 +422,7 @@ describe API::Tags do context 'on tag with existing release' do before do release = project.releases.find_or_initialize_by(tag: tag_name) - release.update_attributes(description: description) + release.update(description: description) end it 'updates the release description' do diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index decb5d22f59..4197277719f 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -49,7 +49,7 @@ describe Ci::RetryBuildService do # Make sure that build has both `stage_id` and `stage` because FactoryBot # can reset one of the fields when assigning another. We plan to deprecate # and remove legacy `stage` column in the future. - build.update_attributes(stage: 'test', stage_id: stage.id) + build.update(stage: 'test', stage_id: stage.id) end describe 'clone accessors' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 158541d36e3..4cbf1a52e29 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -501,7 +501,7 @@ describe Issues::UpdateService, :mailer do let(:params) { { label_ids: [], remove_label_ids: [label.id] } } before do - issue.update_attributes(labels: [label, label3]) + issue.update(labels: [label, label3]) end it 'ignores the label_ids parameter' do @@ -517,7 +517,7 @@ describe Issues::UpdateService, :mailer do let(:params) { { add_label_ids: [label3.id], remove_label_ids: [label.id] } } before do - issue.update_attributes(labels: [label]) + issue.update(labels: [label]) end it 'adds the passed labels' do diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 36b6e5a701e..b4e9f6af43a 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -142,8 +142,8 @@ describe Members::DestroyService do context 'with an access requester' do before do - group_project.update_attributes(request_access_enabled: true) - group.update_attributes(request_access_enabled: true) + group_project.update(request_access_enabled: true) + group.update(request_access_enabled: true) group_project.request_access(member_user) group.request_access(member_user) end diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index 97da8a88660..a5520e7373e 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -34,7 +34,7 @@ describe MergeRequests::Conflicts::ListService do it 'returns a falsey value when the MR does not support new diff notes' do merge_request = create_merge_request('conflict-resolvable') - merge_request.merge_request_diff.update_attributes(start_commit_sha: nil) + merge_request.merge_request_diff.update(start_commit_sha: nil) expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index ef2738ef504..33eea2cf5c3 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -63,7 +63,7 @@ describe MergeRequests::MergeService do let(:commit) { double('commit', safe_message: "Fixes #{jira_issue.to_reference}") } before do - project.update_attributes!(has_external_issue_tracker: true) + project.update!(has_external_issue_tracker: true) jira_service_settings stub_jira_urls(jira_issue.id) allow(merge_request).to receive(:commits).and_return([commit]) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0eadc83bfe3..ab91176737d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -1314,7 +1314,7 @@ describe NotificationService, :mailer do describe 'when merge_when_pipeline_succeeds is true' do before do - merge_request.update_attributes( + merge_request.update( merge_when_pipeline_succeeds: true, merge_user: create(:user) ) diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index c15f5120b8a..f89f9b54f53 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -135,7 +135,7 @@ describe Projects::ForkService do context "when project has restricted visibility level" do context "and only one visibility level is restricted" do before do - @from_project.update_attributes(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + @from_project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) end diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 1bffeee6790..a4c103e6f30 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -24,8 +24,8 @@ describe Projects::UpdatePagesService do let(:extension) { 'zip' } before do - build.update_attributes(legacy_artifacts_file: file) - build.update_attributes(legacy_artifacts_metadata: metadata) + build.update(legacy_artifacts_file: file) + build.update(legacy_artifacts_metadata: metadata) end describe 'pages artifacts' do @@ -62,13 +62,13 @@ describe Projects::UpdatePagesService do end it 'fails if sha on branch is not latest' do - build.update_attributes(ref: 'feature') + build.update(ref: 'feature') expect(execute).not_to eq(:success) end it 'fails for empty file fails' do - build.update_attributes(legacy_artifacts_file: empty_file) + build.update(legacy_artifacts_file: empty_file) expect { execute } .to raise_error(Projects::UpdatePagesService::FailedToExtractError) @@ -118,7 +118,7 @@ describe Projects::UpdatePagesService do end it 'fails if sha on branch is not latest' do - build.update_attributes(ref: 'feature') + build.update(ref: 'feature') expect(execute).not_to eq(:success) end @@ -188,7 +188,7 @@ describe Projects::UpdatePagesService do end it 'fails for invalid archive' do - build.update_attributes(legacy_artifacts_file: invalid_file) + build.update(legacy_artifacts_file: invalid_file) expect(execute).not_to eq(:success) end @@ -199,8 +199,8 @@ describe Projects::UpdatePagesService do file = fixture_file_upload('spec/fixtures/pages.zip') metafile = fixture_file_upload('spec/fixtures/pages.zip.meta') - build.update_attributes(legacy_artifacts_file: file) - build.update_attributes(legacy_artifacts_metadata: metafile) + build.update(legacy_artifacts_file: file) + build.update(legacy_artifacts_metadata: metafile) allow(build).to receive(:artifacts_metadata_entry) .and_return(metadata) diff --git a/spec/services/reset_project_cache_service_spec.rb b/spec/services/reset_project_cache_service_spec.rb index de475d16586..1490ad5fe3b 100644 --- a/spec/services/reset_project_cache_service_spec.rb +++ b/spec/services/reset_project_cache_service_spec.rb @@ -18,7 +18,7 @@ describe ResetProjectCacheService do context 'when project cache_index is a numeric value' do before do - project.update_attributes(jobs_cache_index: 1) + project.update(jobs_cache_index: 1) end it 'increments project cache index' do diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 51396d34f8f..e0335880e8e 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -75,7 +75,7 @@ describe SystemHooksService do end it 'handles nil datetime columns' do - user.update_attributes(created_at: nil, updated_at: nil) + user.update(created_at: nil, updated_at: nil) data = event_data(user, :destroy) expect(data[:created_at]).to be(nil) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 46ec1bcef24..bd564cc60a6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,7 +4,7 @@ SimpleCovEnv.start! ENV["RAILS_ENV"] = 'test' ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true' -require File.expand_path("../../config/environment", __FILE__) +require File.expand_path('../config/environment', __dir__) require 'rspec/rails' require 'shoulda/matchers' require 'rspec/retry' diff --git a/spec/support/api/repositories_shared_context.rb b/spec/support/api/repositories_shared_context.rb index ea38fe4f5b8..f1341804e56 100644 --- a/spec/support/api/repositories_shared_context.rb +++ b/spec/support/api/repositories_shared_context.rb @@ -1,6 +1,6 @@ shared_context 'disabled repository' do before do - project.project_feature.update_attributes!( + project.project_feature.update!( repository_access_level: ProjectFeature::DISABLED, merge_requests_access_level: ProjectFeature::DISABLED, builds_access_level: ProjectFeature::DISABLED diff --git a/spec/support/api/time_tracking_shared_examples.rb b/spec/support/api/time_tracking_shared_examples.rb index 52e1bc55191..fee464c15a3 100644 --- a/spec/support/api/time_tracking_shared_examples.rb +++ b/spec/support/api/time_tracking_shared_examples.rb @@ -85,7 +85,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| it 'subtracts time of the total spent time' do Timecop.travel(1.minute.from_now) do expect do - issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id }) + issuable.update!(spend_time: { duration: 7200, user_id: user.id }) end.to change { issuable.reload.updated_at } end @@ -99,7 +99,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| context 'when time to subtract is greater than the total spent time' do it 'does not modify the total time spent' do - issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id }) + issuable.update!(spend_time: { duration: 7200, user_id: user.id }) Timecop.travel(1.minute.from_now) do expect do @@ -135,8 +135,8 @@ shared_examples 'time tracking endpoints' do |issuable_name| describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do it "returns the time stats for #{issuable_name}" do - issuable.update_attributes!(spend_time: { duration: 1800, user_id: user.id }, - time_estimate: 3600) + issuable.update!(spend_time: { duration: 1800, user_id: user.id }, + time_estimate: 3600) get api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_stats", user) diff --git a/spec/support/generate-seed-repo-rb b/spec/support/generate-seed-repo-rb index 44b3de23b99..bee9d419376 100755 --- a/spec/support/generate-seed-repo-rb +++ b/spec/support/generate-seed-repo-rb @@ -15,7 +15,7 @@ require 'erb' require 'tempfile' -SOURCE = File.expand_path('../gitlab-git-test.git', __FILE__).freeze +SOURCE = File.expand_path('gitlab-git-test.git', __dir__).freeze SCRIPT_NAME = 'generate-seed-repo-rb'.freeze REPO_NAME = 'gitlab-git-test.git'.freeze diff --git a/spec/support/helpers/jira_service_helper.rb b/spec/support/helpers/jira_service_helper.rb index 88a7aeba461..f4d5343c4ed 100644 --- a/spec/support/helpers/jira_service_helper.rb +++ b/spec/support/helpers/jira_service_helper.rb @@ -12,7 +12,7 @@ module JiraServiceHelper jira_issue_transition_id: '1' } - jira_tracker.update_attributes(properties: properties, active: true) + jira_tracker.update(properties: properties, active: true) end def jira_issue_comments diff --git a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb index 7c34c7b4977..940c24c8d67 100644 --- a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb @@ -130,7 +130,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do context "event channels" do it "uses the right channel for push event" do - chat_service.update_attributes(push_channel: "random") + chat_service.update(push_channel: "random") expect(Slack::Notifier).to receive(:new) .with(webhook_url, channel: "random") @@ -142,7 +142,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do end it "uses the right channel for merge request event" do - chat_service.update_attributes(merge_request_channel: "random") + chat_service.update(merge_request_channel: "random") expect(Slack::Notifier).to receive(:new) .with(webhook_url, channel: "random") @@ -154,7 +154,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do end it "uses the right channel for issue event" do - chat_service.update_attributes(issue_channel: "random") + chat_service.update(issue_channel: "random") expect(Slack::Notifier).to receive(:new) .with(webhook_url, channel: "random") @@ -169,7 +169,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do let(:issue_service_options) { { title: 'Secret', confidential: true } } it "uses confidential issue channel" do - chat_service.update_attributes(confidential_issue_channel: 'confidential') + chat_service.update(confidential_issue_channel: 'confidential') expect(Slack::Notifier).to execute_with_options(channel: 'confidential') @@ -177,7 +177,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do end it 'falls back to issue channel' do - chat_service.update_attributes(issue_channel: 'fallback_channel') + chat_service.update(issue_channel: 'fallback_channel') expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel') @@ -186,7 +186,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do end it "uses the right channel for wiki event" do - chat_service.update_attributes(wiki_page_channel: "random") + chat_service.update(wiki_page_channel: "random") expect(Slack::Notifier).to receive(:new) .with(webhook_url, channel: "random") @@ -203,7 +203,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do end it "uses the right channel" do - chat_service.update_attributes(note_channel: "random") + chat_service.update(note_channel: "random") note_data = Gitlab::DataBuilder::Note.build(issue_note, user) @@ -222,7 +222,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do end it "uses confidential channel" do - chat_service.update_attributes(confidential_note_channel: "confidential") + chat_service.update(confidential_note_channel: "confidential") note_data = Gitlab::DataBuilder::Note.build(issue_note, user) @@ -232,7 +232,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do end it 'falls back to note channel' do - chat_service.update_attributes(note_channel: "fallback_channel") + chat_service.update(note_channel: "fallback_channel") note_data = Gitlab::DataBuilder::Note.build(issue_note, user) diff --git a/spec/views/projects/imports/new.html.haml_spec.rb b/spec/views/projects/imports/new.html.haml_spec.rb index 32d73d0c5ab..fc389641fcd 100644 --- a/spec/views/projects/imports/new.html.haml_spec.rb +++ b/spec/views/projects/imports/new.html.haml_spec.rb @@ -7,7 +7,7 @@ describe "projects/imports/new.html.haml" do let(:project) { create(:project_empty_repo, :import_failed, import_type: :gitlab_project, import_source: '/var/opt/gitlab/gitlab-rails/shared/tmp/project_exports/uploads/t.tar.gz', import_url: nil) } before do - project.import_state.update_attributes(last_error: 'Foo') + project.import_state.update(last_error: 'Foo') sign_in(user) project.add_master(user) end diff --git a/spec/views/projects/merge_requests/show.html.haml_spec.rb b/spec/views/projects/merge_requests/show.html.haml_spec.rb index 264e0ce0b40..fe6ad26a6f6 100644 --- a/spec/views/projects/merge_requests/show.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/show.html.haml_spec.rb @@ -52,7 +52,7 @@ describe 'projects/merge_requests/show.html.haml' do context 'when the merge request is open' do it 'closes the merge request if the source project does not exist' do - closed_merge_request.update_attributes(state: 'open') + closed_merge_request.update(state: 'open') forked_project.destroy # Reload merge request so MergeRequest#source_project turns to `nil` closed_merge_request.reload diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index f0884ad0aff..d07e40377d4 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -51,7 +51,7 @@ describe RepositoryImportWorker do it 'hide the credentials that were used in the import URL' do error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found } - project.update_attributes(import_jid: '123') + project.update(import_jid: '123') expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error }) expect do @@ -63,7 +63,7 @@ describe RepositoryImportWorker do it 'updates the error on Import/Export' do error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found } - project.update_attributes(import_jid: '123', import_type: 'gitlab_project') + project.update(import_jid: '123', import_type: 'gitlab_project') expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error }) expect do diff --git a/spec/workers/repository_update_remote_mirror_worker_spec.rb b/spec/workers/repository_update_remote_mirror_worker_spec.rb index 152ba2509b9..4f1ad2474f5 100644 --- a/spec/workers/repository_update_remote_mirror_worker_spec.rb +++ b/spec/workers/repository_update_remote_mirror_worker_spec.rb @@ -13,7 +13,7 @@ describe RepositoryUpdateRemoteMirrorWorker do describe '#perform' do context 'with status none' do before do - remote_mirror.update_attributes(update_status: 'none') + remote_mirror.update(update_status: 'none') end it 'sets status as finished when update remote mirror service executes successfully' do @@ -34,7 +34,7 @@ describe RepositoryUpdateRemoteMirrorWorker do end it 'does nothing if last_update_started_at is higher than the time the job was scheduled in' do - remote_mirror.update_attributes(last_update_started_at: Time.now) + remote_mirror.update(last_update_started_at: Time.now) expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(true) expect_any_instance_of(Projects::UpdateRemoteMirrorService).not_to receive(:execute).with(remote_mirror) @@ -56,7 +56,7 @@ describe RepositoryUpdateRemoteMirrorWorker do context 'with another worker already running' do before do - remote_mirror.update_attributes(update_status: 'started') + remote_mirror.update(update_status: 'started') end it 'raises RemoteMirrorUpdateAlreadyInProgressError' do @@ -68,11 +68,11 @@ describe RepositoryUpdateRemoteMirrorWorker do context 'with status failed' do before do - remote_mirror.update_attributes(update_status: 'failed') + remote_mirror.update(update_status: 'failed') end it 'sets status as finished if last_update_started_at is higher than the time the job was scheduled in' do - remote_mirror.update_attributes(last_update_started_at: Time.now) + remote_mirror.update(last_update_started_at: Time.now) expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(false) expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) diff --git a/spec/workers/stuck_import_jobs_worker_spec.rb b/spec/workers/stuck_import_jobs_worker_spec.rb index af7675c8cab..2169c14218b 100644 --- a/spec/workers/stuck_import_jobs_worker_spec.rb +++ b/spec/workers/stuck_import_jobs_worker_spec.rb @@ -51,7 +51,7 @@ describe StuckImportJobsWorker do let(:project) { create(:project, :import_scheduled) } before do - project.import_state.update_attributes(jid: '123') + project.import_state.update(jid: '123') end end end @@ -61,7 +61,7 @@ describe StuckImportJobsWorker do let(:project) { create(:project, :import_started) } before do - project.import_state.update_attributes(jid: '123') + project.import_state.update(jid: '123') end end end From 9286f5b9340f92131c320c231a5fb3e51c23bf04 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 2 Jul 2018 13:57:38 +0000 Subject: [PATCH 17/38] Use stable gitlab-styles and eliminate offenses --- .rubocop.yml | 6 ++++++ Gemfile | 2 +- Gemfile.lock | 16 +++++----------- Gemfile.rails5.lock | 16 +++++----------- app/controllers/projects/wikis_controller.rb | 2 +- app/models/wiki_page.rb | 5 +++-- app/services/notification_recipient_service.rb | 1 + .../lfs_pointers/lfs_download_service.rb | 2 +- lib/gitlab/git/repository.rb | 2 +- lib/gitlab/metrics/influx_db.rb | 1 - .../projects/files/user_browses_files_spec.rb | 1 - spec/helpers/blob_helper_spec.rb | 4 ++-- spec/lib/gitlab/sanitizers/svg_spec.rb | 4 ++-- spec/lib/gitlab/workhorse_spec.rb | 2 +- 14 files changed, 29 insertions(+), 35 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 0582bfe8d70..d0584b46b06 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -31,6 +31,12 @@ Style/MutableConstant: - 'ee/db/post_migrate/**/*' - 'ee/db/geo/migrate/**/*' +Naming/UncommunicativeMethodParamName: + Enabled: false + +Naming/MemoizedInstanceVariableName: + Enabled: false + Naming/FileName: ExpectMatchingDefinition: true Exclude: diff --git a/Gemfile b/Gemfile index e52627dbadd..b83bdeb15e5 100644 --- a/Gemfile +++ b/Gemfile @@ -351,7 +351,7 @@ group :development, :test do gem 'spring', '~> 2.0.0' gem 'spring-commands-rspec', '~> 1.0.4' - gem 'gitlab-styles', '~> 2.4', require: false, git: 'https://gitlab.com/gitlab-org/gitlab-styles.git', branch: 'update-to-0.54.0' + gem 'gitlab-styles', '~> 2.4', require: false # Pin these dependencies, otherwise a new rule could break the CI pipelines gem 'rubocop', '~> 0.54.0' gem 'rubocop-rspec', '~> 1.22.1' diff --git a/Gemfile.lock b/Gemfile.lock index 41d22f3ec89..ad0675ddb8f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,13 +1,3 @@ -GIT - remote: https://gitlab.com/gitlab-org/gitlab-styles.git - revision: f3250343d3a13a154fe9b2d356440654297b0624 - branch: update-to-0.54.0 - specs: - gitlab-styles (2.4.0) - rubocop (~> 0.54.0) - rubocop-gitlab-security (~> 0.1.0) - rubocop-rspec (~> 1.19) - GEM remote: https://rubygems.org/ specs: @@ -322,6 +312,10 @@ GEM mime-types (>= 1.16) posix-spawn (~> 0.3) gitlab-markup (1.6.4) + gitlab-styles (2.4.0) + rubocop (~> 0.54.0) + rubocop-gitlab-security (~> 0.1.0) + rubocop-rspec (~> 1.19) gitlab_omniauth-ldap (2.0.4) net-ldap (~> 0.16) omniauth (~> 1.3) @@ -1049,7 +1043,7 @@ DEPENDENCIES gitlab-gollum-lib (~> 4.2) gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.4) - gitlab-styles (~> 2.4)! + gitlab-styles (~> 2.4) gitlab_omniauth-ldap (~> 2.0.4) gon (~> 6.2) google-api-client (~> 0.19.8) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index b74cbf41343..e167bbcb52e 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -1,13 +1,3 @@ -GIT - remote: https://gitlab.com/gitlab-org/gitlab-styles.git - revision: f3250343d3a13a154fe9b2d356440654297b0624 - branch: update-to-0.54.0 - specs: - gitlab-styles (2.4.0) - rubocop (~> 0.54.0) - rubocop-gitlab-security (~> 0.1.0) - rubocop-rspec (~> 1.19) - GEM remote: https://rubygems.org/ specs: @@ -325,6 +315,10 @@ GEM mime-types (>= 1.16) posix-spawn (~> 0.3) gitlab-markup (1.6.4) + gitlab-styles (2.4.0) + rubocop (~> 0.54.0) + rubocop-gitlab-security (~> 0.1.0) + rubocop-rspec (~> 1.19) gitlab_omniauth-ldap (2.0.4) net-ldap (~> 0.16) omniauth (~> 1.3) @@ -1059,7 +1053,7 @@ DEPENDENCIES gitlab-gollum-lib (~> 4.2) gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.4) - gitlab-styles (~> 2.4)! + gitlab-styles (~> 2.4) gitlab_omniauth-ldap (~> 2.0.4) gon (~> 6.2) google-api-client (~> 0.19.8) diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 658b6ee5820..c01066c688a 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -129,7 +129,7 @@ class Projects::WikisController < Projects::ApplicationController def build_page(args) WikiPage.new(@project_wiki).tap do |page| - page.update(args) + page.update_attributes(args) # rubocop:disable Rails/ActiveRecordAliases end end end diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 6cd3993a56d..4b49edb01a5 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -1,3 +1,4 @@ +# rubocop:disable Rails/ActiveRecordAliases class WikiPage PageChangedError = Class.new(StandardError) PageRenameError = Class.new(StandardError) @@ -190,7 +191,7 @@ class WikiPage # Returns the String SHA1 of the newly created page # or False if the save was unsuccessful. def create(attrs = {}) - update(attrs) + update_attributes(attrs) save(page_details: title) do wiki.create_page(title, content, format, message) @@ -216,7 +217,7 @@ class WikiPage raise PageChangedError end - update(attrs) + update_attributes(attrs) if title_changed? page_details = title diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index d3447aa9951..628215b55fa 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -43,6 +43,7 @@ module NotificationRecipientService def target raise 'abstract' end + def project target.project end diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index 6ea43561d61..618c30b971f 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -22,7 +22,7 @@ module Projects private def download_and_save_file(file, sanitized_uri) - IO.copy_stream(open(sanitized_uri.sanitized_url, headers(sanitized_uri)), file) + IO.copy_stream(open(sanitized_uri.sanitized_url, headers(sanitized_uri)), file) # rubocop:disable Security/Open end def headers(sanitized_uri) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 324eac038e1..ef29a871be0 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1068,6 +1068,7 @@ module Gitlab true end + def multi_action( user, branch_name:, message:, actions:, author_email: nil, author_name: nil, @@ -1079,7 +1080,6 @@ module Gitlab start_branch_name, start_repository) end end - # rubocop:enable Metrics/ParameterLists def write_config(full_path:) return unless full_path.present? diff --git a/lib/gitlab/metrics/influx_db.rb b/lib/gitlab/metrics/influx_db.rb index 93cd6580d9a..04135dac4ff 100644 --- a/lib/gitlab/metrics/influx_db.rb +++ b/lib/gitlab/metrics/influx_db.rb @@ -162,7 +162,6 @@ module Gitlab # When enabled this should be set before being used as the usual pattern # "@foo ||= bar" is _not_ thread-safe. - # rubocop:disable Gitlab/ModuleWithInstanceVariables def pool if influx_metrics_enabled? if @pool.nil? diff --git a/spec/features/projects/files/user_browses_files_spec.rb b/spec/features/projects/files/user_browses_files_spec.rb index 9b1395d5e0a..f56174fc85c 100644 --- a/spec/features/projects/files/user_browses_files_spec.rb +++ b/spec/features/projects/files/user_browses_files_spec.rb @@ -149,7 +149,6 @@ describe "User browses files" do end # Test the full URLs of links instead of relative paths by `have_link(text: "...", href: "...")`. find("a", text: /^empty$/)["href"] == project_blob_url(project, "markdown/d/README.md") - # rubocop:enable Lint/Void end it "shows correct content of directory" do diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index a3e010c3206..1c216b3fe97 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -65,9 +65,9 @@ describe BlobHelper do describe "#sanitize_svg_data" do let(:input_svg_path) { File.join(Rails.root, 'spec', 'fixtures', 'unsanitized.svg') } - let(:data) { open(input_svg_path).read } + let(:data) { File.read(input_svg_path) } let(:expected_svg_path) { File.join(Rails.root, 'spec', 'fixtures', 'sanitized.svg') } - let(:expected) { open(expected_svg_path).read } + let(:expected) { File.read(expected_svg_path) } it 'retains essential elements' do expect(sanitize_svg_data(data)).to eq(expected) diff --git a/spec/lib/gitlab/sanitizers/svg_spec.rb b/spec/lib/gitlab/sanitizers/svg_spec.rb index 030c2063ab2..df46a874528 100644 --- a/spec/lib/gitlab/sanitizers/svg_spec.rb +++ b/spec/lib/gitlab/sanitizers/svg_spec.rb @@ -7,9 +7,9 @@ describe Gitlab::Sanitizers::SVG do describe '.clean' do let(:input_svg_path) { File.join(Rails.root, 'spec', 'fixtures', 'unsanitized.svg') } - let(:data) { open(input_svg_path).read } + let(:data) { File.read(input_svg_path) } let(:sanitized_svg_path) { File.join(Rails.root, 'spec', 'fixtures', 'sanitized.svg') } - let(:sanitized) { open(sanitized_svg_path).read } + let(:sanitized) { File.read(sanitized_svg_path) } it 'delegates sanitization to scrubber' do expect_any_instance_of(Gitlab::Sanitizers::SVG::Scrubber).to receive(:scrub).at_least(:once) diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 8fdcfe79fb5..7802ff1f5f6 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -177,7 +177,7 @@ describe Gitlab::Workhorse do end it 'accepts a trailing newline' do - open(described_class.secret_path, 'a') { |f| f.write "\n" } + File.open(described_class.secret_path, 'a') { |f| f.write "\n" } expect(subject.length).to eq(32) end From 3bfe30662436e48d4324b8fac5c1634df4a7cf7f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 4 Jul 2018 22:02:01 +0800 Subject: [PATCH 18/38] Resolve Naming/UncommunicativeMethod --- app/models/network/commit.rb | 4 +- app/models/repository.rb | 10 +- app/models/user.rb | 4 +- .../notification_recipient_service.rb | 12 +- app/workers/email_receiver_worker.rb | 8 +- lib/declarative_policy/base.rb | 10 +- lib/declarative_policy/delegate_dsl.rb | 6 +- lib/declarative_policy/policy_dsl.rb | 14 +-- lib/declarative_policy/preferred_scope.rb | 10 +- lib/declarative_policy/rule.rb | 4 +- lib/declarative_policy/rule_dsl.rb | 10 +- lib/declarative_policy/runner.rb | 2 +- lib/gitlab/ci/ansi2html.rb | 118 +++++++++--------- lib/gitlab/ci/trace/section_parser.rb | 12 +- lib/gitlab/diff/image_point.rb | 6 +- lib/gitlab/diff/inline_diff.rb | 4 +- lib/gitlab/encoding_helper.rb | 10 +- lib/gitlab/fogbugz_import/importer.rb | 22 ++-- lib/gitlab/gitaly_client.rb | 4 +- lib/gitlab/gitaly_client/commit_service.rb | 4 +- lib/gitlab/gitaly_client/storage_settings.rb | 4 +- lib/gitlab/google_code_import/importer.rb | 22 ++-- lib/rouge/formatters/html_gitlab.rb | 2 +- ...ters_to_new_clusters_architectures_spec.rb | 4 +- spec/support/helpers/key_generator_helper.rb | 2 +- spec/workers/concerns/waitable_worker_spec.rb | 4 +- 26 files changed, 156 insertions(+), 156 deletions(-) diff --git a/app/models/network/commit.rb b/app/models/network/commit.rb index 22d48c9e661..d667948deae 100644 --- a/app/models/network/commit.rb +++ b/app/models/network/commit.rb @@ -11,8 +11,8 @@ module Network @parent_spaces = [] end - def method_missing(m, *args, &block) - @commit.__send__(m, *args, &block) # rubocop:disable GitlabSecurity/PublicSend + def method_missing(msg, *args, &block) + @commit.__send__(msg, *args, &block) # rubocop:disable GitlabSecurity/PublicSend end def space diff --git a/app/models/repository.rb b/app/models/repository.rb index 7cd600fec5b..5ed2a7b4068 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -462,12 +462,12 @@ class Repository expire_branches_cache end - def method_missing(m, *args, &block) - if m == :lookup && !block_given? - lookup_cache[m] ||= {} - lookup_cache[m][args.join(":")] ||= raw_repository.__send__(m, *args, &block) # rubocop:disable GitlabSecurity/PublicSend + def method_missing(msg, *args, &block) + if msg == :lookup && !block_given? + lookup_cache[msg] ||= {} + lookup_cache[msg][args.join(":")] ||= raw_repository.__send__(msg, *args, &block) # rubocop:disable GitlabSecurity/PublicSend else - raw_repository.__send__(m, *args, &block) # rubocop:disable GitlabSecurity/PublicSend + raw_repository.__send__(msg, *args, &block) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/models/user.rb b/app/models/user.rb index 47542ab122a..1c5d39db118 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1333,8 +1333,8 @@ class User < ActiveRecord::Base end end - def self.unique_internal(scope, username, email_pattern, &b) - scope.first || create_unique_internal(scope, username, email_pattern, &b) + def self.unique_internal(scope, username, email_pattern, &block) + scope.first || create_unique_internal(scope, username, email_pattern, &block) end def self.create_unique_internal(scope, username, email_pattern, &creation_block) diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 628215b55fa..d9834fd0ccc 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -10,16 +10,16 @@ module NotificationRecipientService NotificationRecipient.new(user, *args).notifiable? end - def self.build_recipients(*a) - Builder::Default.new(*a).notification_recipients + def self.build_recipients(*args) + Builder::Default.new(*args).notification_recipients end - def self.build_new_note_recipients(*a) - Builder::NewNote.new(*a).notification_recipients + def self.build_new_note_recipients(*args) + Builder::NewNote.new(*args).notification_recipients end - def self.build_merge_request_unmergeable_recipients(*a) - Builder::MergeRequestUnmergeable.new(*a).notification_recipients + def self.build_merge_request_unmergeable_recipients(*args) + Builder::MergeRequestUnmergeable.new(*args).notification_recipients end module Builder diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index f9f0efb302a..12706613ac2 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -15,14 +15,14 @@ class EmailReceiverWorker private - def handle_failure(raw, e) - Rails.logger.warn("Email can not be processed: #{e}\n\n#{raw}") + def handle_failure(raw, error) + Rails.logger.warn("Email can not be processed: #{error}\n\n#{raw}") return unless raw.present? can_retry = false reason = - case e + case error when Gitlab::Email::UnknownIncomingEmail "We couldn't figure out what the email is for. Please create your issue or comment through the web interface." when Gitlab::Email::SentNotificationNotFoundError @@ -42,7 +42,7 @@ class EmailReceiverWorker "The thread you are replying to no longer exists, perhaps it was deleted? If you believe this is in error, contact a staff member." when Gitlab::Email::InvalidRecordError can_retry = true - e.message + error.message end if reason diff --git a/lib/declarative_policy/base.rb b/lib/declarative_policy/base.rb index 47542194497..da3fabba39b 100644 --- a/lib/declarative_policy/base.rb +++ b/lib/declarative_policy/base.rb @@ -119,8 +119,8 @@ module DeclarativePolicy # a PolicyDsl which is used for registering the rule with # this class. PolicyDsl will call back into Base.enable_when, # Base.prevent_when, and Base.prevent_all_when. - def rule(&b) - rule = RuleDsl.new(self).instance_eval(&b) + def rule(&block) + rule = RuleDsl.new(self).instance_eval(&block) PolicyDsl.new(self, rule) end @@ -222,8 +222,8 @@ module DeclarativePolicy # computes the given ability and prints a helpful debugging output # showing which - def debug(ability, *a) - runner(ability).debug(*a) + def debug(ability, *args) + runner(ability).debug(*args) end desc "Unknown user" @@ -274,7 +274,7 @@ module DeclarativePolicy # # NOTE we can't use ||= here because the value might be the # boolean `false` - def cache(key, &b) + def cache(key) return @cache[key] if cached?(key) @cache[key] = yield diff --git a/lib/declarative_policy/delegate_dsl.rb b/lib/declarative_policy/delegate_dsl.rb index f544dffe888..ca2eb98e3e8 100644 --- a/lib/declarative_policy/delegate_dsl.rb +++ b/lib/declarative_policy/delegate_dsl.rb @@ -7,10 +7,10 @@ module DeclarativePolicy @delegate_name = delegate_name end - def method_missing(m, *a, &b) - return super unless a.empty? && !block_given? + def method_missing(msg, *args) + return super unless args.empty? && !block_given? - @rule_dsl.delegate(@delegate_name, m) + @rule_dsl.delegate(@delegate_name, msg) end end end diff --git a/lib/declarative_policy/policy_dsl.rb b/lib/declarative_policy/policy_dsl.rb index f11b6e9f730..c96049768a1 100644 --- a/lib/declarative_policy/policy_dsl.rb +++ b/lib/declarative_policy/policy_dsl.rb @@ -15,8 +15,8 @@ module DeclarativePolicy @rule = rule end - def policy(&b) - instance_eval(&b) + def policy(&block) + instance_eval(&block) end def enable(*abilities) @@ -31,14 +31,14 @@ module DeclarativePolicy @context_class.prevent_all_when(@rule) end - def method_missing(m, *a, &b) - return super unless @context_class.respond_to?(m) + def method_missing(msg, *args, &block) + return super unless @context_class.respond_to?(msg) - @context_class.__send__(m, *a, &b) # rubocop:disable GitlabSecurity/PublicSend + @context_class.__send__(msg, *args, &block) # rubocop:disable GitlabSecurity/PublicSend end - def respond_to_missing?(m) - @context_class.respond_to?(m) || super + def respond_to_missing?(msg) + @context_class.respond_to?(msg) || super end end end diff --git a/lib/declarative_policy/preferred_scope.rb b/lib/declarative_policy/preferred_scope.rb index 5c214408dd0..c77784cb49d 100644 --- a/lib/declarative_policy/preferred_scope.rb +++ b/lib/declarative_policy/preferred_scope.rb @@ -2,7 +2,7 @@ module DeclarativePolicy # rubocop:disable Naming/FileName PREFERRED_SCOPE_KEY = :"DeclarativePolicy.preferred_scope" class << self - def with_preferred_scope(scope, &b) + def with_preferred_scope(scope) Thread.current[PREFERRED_SCOPE_KEY], old_scope = scope, Thread.current[PREFERRED_SCOPE_KEY] yield ensure @@ -13,12 +13,12 @@ module DeclarativePolicy # rubocop:disable Naming/FileName Thread.current[PREFERRED_SCOPE_KEY] end - def user_scope(&b) - with_preferred_scope(:user, &b) + def user_scope(&block) + with_preferred_scope(:user, &block) end - def subject_scope(&b) - with_preferred_scope(:subject, &b) + def subject_scope(&block) + with_preferred_scope(:subject, &block) end def preferred_scope=(scope) diff --git a/lib/declarative_policy/rule.rb b/lib/declarative_policy/rule.rb index e309244a3b3..407398cc770 100644 --- a/lib/declarative_policy/rule.rb +++ b/lib/declarative_policy/rule.rb @@ -8,8 +8,8 @@ module DeclarativePolicy # how that affects the actual ability decision - for that, a # `Step` is used. class Base - def self.make(*a) - new(*a).simplify + def self.make(*args) + new(*args).simplify end # true or false whether this rule passes. diff --git a/lib/declarative_policy/rule_dsl.rb b/lib/declarative_policy/rule_dsl.rb index e948b7f2de1..7254b08eda5 100644 --- a/lib/declarative_policy/rule_dsl.rb +++ b/lib/declarative_policy/rule_dsl.rb @@ -32,13 +32,13 @@ module DeclarativePolicy Rule::DelegatedCondition.new(delegate_name, condition) end - def method_missing(m, *a, &b) - return super unless a.empty? && !block_given? + def method_missing(msg, *args) + return super unless args.empty? && !block_given? - if @context_class.delegations.key?(m) - DelegateDsl.new(self, m) + if @context_class.delegations.key?(msg) + DelegateDsl.new(self, msg) else - cond(m.to_sym) + cond(msg.to_sym) end end end diff --git a/lib/declarative_policy/runner.rb b/lib/declarative_policy/runner.rb index 87f14b3b0d2..fec672f4b8c 100644 --- a/lib/declarative_policy/runner.rb +++ b/lib/declarative_policy/runner.rb @@ -127,7 +127,7 @@ module DeclarativePolicy # # For each step, we yield the step object along with the computed score # for debugging purposes. - def steps_by_score(&b) + def steps_by_score flatten_steps! if @steps.size > 50 diff --git a/lib/gitlab/ci/ansi2html.rb b/lib/gitlab/ci/ansi2html.rb index 35eadf6fa93..e780f8c646b 100644 --- a/lib/gitlab/ci/ansi2html.rb +++ b/lib/gitlab/ci/ansi2html.rb @@ -29,105 +29,105 @@ module Gitlab end class Converter - def on_0(s) reset() end + def on_0(_) reset() end - def on_1(s) enable(STYLE_SWITCHES[:bold]) end + def on_1(_) enable(STYLE_SWITCHES[:bold]) end - def on_3(s) enable(STYLE_SWITCHES[:italic]) end + def on_3(_) enable(STYLE_SWITCHES[:italic]) end - def on_4(s) enable(STYLE_SWITCHES[:underline]) end + def on_4(_) enable(STYLE_SWITCHES[:underline]) end - def on_8(s) enable(STYLE_SWITCHES[:conceal]) end + def on_8(_) enable(STYLE_SWITCHES[:conceal]) end - def on_9(s) enable(STYLE_SWITCHES[:cross]) end + def on_9(_) enable(STYLE_SWITCHES[:cross]) end - def on_21(s) disable(STYLE_SWITCHES[:bold]) end + def on_21(_) disable(STYLE_SWITCHES[:bold]) end - def on_22(s) disable(STYLE_SWITCHES[:bold]) end + def on_22(_) disable(STYLE_SWITCHES[:bold]) end - def on_23(s) disable(STYLE_SWITCHES[:italic]) end + def on_23(_) disable(STYLE_SWITCHES[:italic]) end - def on_24(s) disable(STYLE_SWITCHES[:underline]) end + def on_24(_) disable(STYLE_SWITCHES[:underline]) end - def on_28(s) disable(STYLE_SWITCHES[:conceal]) end + def on_28(_) disable(STYLE_SWITCHES[:conceal]) end - def on_29(s) disable(STYLE_SWITCHES[:cross]) end + def on_29(_) disable(STYLE_SWITCHES[:cross]) end - def on_30(s) set_fg_color(0) end + def on_30(_) set_fg_color(0) end - def on_31(s) set_fg_color(1) end + def on_31(_) set_fg_color(1) end - def on_32(s) set_fg_color(2) end + def on_32(_) set_fg_color(2) end - def on_33(s) set_fg_color(3) end + def on_33(_) set_fg_color(3) end - def on_34(s) set_fg_color(4) end + def on_34(_) set_fg_color(4) end - def on_35(s) set_fg_color(5) end + def on_35(_) set_fg_color(5) end - def on_36(s) set_fg_color(6) end + def on_36(_) set_fg_color(6) end - def on_37(s) set_fg_color(7) end + def on_37(_) set_fg_color(7) end - def on_38(s) set_fg_color_256(s) end + def on_38(stack) set_fg_color_256(stack) end - def on_39(s) set_fg_color(9) end + def on_39(_) set_fg_color(9) end - def on_40(s) set_bg_color(0) end + def on_40(_) set_bg_color(0) end - def on_41(s) set_bg_color(1) end + def on_41(_) set_bg_color(1) end - def on_42(s) set_bg_color(2) end + def on_42(_) set_bg_color(2) end - def on_43(s) set_bg_color(3) end + def on_43(_) set_bg_color(3) end - def on_44(s) set_bg_color(4) end + def on_44(_) set_bg_color(4) end - def on_45(s) set_bg_color(5) end + def on_45(_) set_bg_color(5) end - def on_46(s) set_bg_color(6) end + def on_46(_) set_bg_color(6) end - def on_47(s) set_bg_color(7) end + def on_47(_) set_bg_color(7) end - def on_48(s) set_bg_color_256(s) end + def on_48(stack) set_bg_color_256(stack) end - def on_49(s) set_bg_color(9) end + def on_49(_) set_bg_color(9) end - def on_90(s) set_fg_color(0, 'l') end + def on_90(_) set_fg_color(0, 'l') end - def on_91(s) set_fg_color(1, 'l') end + def on_91(_) set_fg_color(1, 'l') end - def on_92(s) set_fg_color(2, 'l') end + def on_92(_) set_fg_color(2, 'l') end - def on_93(s) set_fg_color(3, 'l') end + def on_93(_) set_fg_color(3, 'l') end - def on_94(s) set_fg_color(4, 'l') end + def on_94(_) set_fg_color(4, 'l') end - def on_95(s) set_fg_color(5, 'l') end + def on_95(_) set_fg_color(5, 'l') end - def on_96(s) set_fg_color(6, 'l') end + def on_96(_) set_fg_color(6, 'l') end - def on_97(s) set_fg_color(7, 'l') end + def on_97(_) set_fg_color(7, 'l') end - def on_99(s) set_fg_color(9, 'l') end + def on_99(_) set_fg_color(9, 'l') end - def on_100(s) set_bg_color(0, 'l') end + def on_100(_) set_bg_color(0, 'l') end - def on_101(s) set_bg_color(1, 'l') end + def on_101(_) set_bg_color(1, 'l') end - def on_102(s) set_bg_color(2, 'l') end + def on_102(_) set_bg_color(2, 'l') end - def on_103(s) set_bg_color(3, 'l') end + def on_103(_) set_bg_color(3, 'l') end - def on_104(s) set_bg_color(4, 'l') end + def on_104(_) set_bg_color(4, 'l') end - def on_105(s) set_bg_color(5, 'l') end + def on_105(_) set_bg_color(5, 'l') end - def on_106(s) set_bg_color(6, 'l') end + def on_106(_) set_bg_color(6, 'l') end - def on_107(s) set_bg_color(7, 'l') end + def on_107(_) set_bg_color(7, 'l') end - def on_109(s) set_bg_color(9, 'l') end + def on_109(_) set_bg_color(9, 'l') end attr_accessor :offset, :n_open_tags, :fg_color, :bg_color, :style_mask @@ -188,19 +188,19 @@ module Gitlab ) end - def handle_section(s) - action = s[1] - timestamp = s[2] - section = s[3] - line = s.matched()[0...-5] # strips \r\033[0K + def handle_section(scanner) + action = scanner[1] + timestamp = scanner[2] + section = scanner[3] + line = scanner.matched()[0...-5] # strips \r\033[0K @out << %{} end - def handle_sequence(s) - indicator = s[1] - commands = s[2].split ';' - terminator = s[3] + def handle_sequence(scanner) + indicator = scanner[1] + commands = scanner[2].split ';' + terminator = scanner[3] # We are only interested in color and text style changes - triggered by # sequences starting with '\e[' and ending with 'm'. Any other control diff --git a/lib/gitlab/ci/trace/section_parser.rb b/lib/gitlab/ci/trace/section_parser.rb index 9bb0166c9e3..c09089d6475 100644 --- a/lib/gitlab/ci/trace/section_parser.rb +++ b/lib/gitlab/ci/trace/section_parser.rb @@ -75,19 +75,19 @@ module Gitlab @beginning_of_section_regex ||= /section_/.freeze end - def find_next_marker(s) + def find_next_marker(scanner) beginning_of_section_len = 8 - maybe_marker = s.exist?(beginning_of_section_regex) + maybe_marker = scanner.exist?(beginning_of_section_regex) if maybe_marker.nil? - s.terminate + scanner.terminate else # repositioning at the beginning of the match - s.pos += maybe_marker - beginning_of_section_len + scanner.pos += maybe_marker - beginning_of_section_len if block_given? - good_marker = yield(s) + good_marker = yield(scanner) # if not a good marker: Consuming the matched beginning_of_section_regex - s.pos += beginning_of_section_len unless good_marker + scanner.pos += beginning_of_section_len unless good_marker end end end diff --git a/lib/gitlab/diff/image_point.rb b/lib/gitlab/diff/image_point.rb index 65332dfd239..1f157354ea4 100644 --- a/lib/gitlab/diff/image_point.rb +++ b/lib/gitlab/diff/image_point.rb @@ -3,11 +3,11 @@ module Gitlab class ImagePoint attr_reader :width, :height, :x, :y - def initialize(width, height, x, y) + def initialize(width, height, new_x, new_y) @width = width @height = height - @x = x - @y = y + @x = new_x + @y = new_y end def to_h diff --git a/lib/gitlab/diff/inline_diff.rb b/lib/gitlab/diff/inline_diff.rb index 54783a07919..99970779c67 100644 --- a/lib/gitlab/diff/inline_diff.rb +++ b/lib/gitlab/diff/inline_diff.rb @@ -93,7 +93,7 @@ module Gitlab private - def longest_common_prefix(a, b) + def longest_common_prefix(a, b) # rubocop:disable Naming/UncommunicativeMethodParamName max_length = [a.length, b.length].max length = 0 @@ -109,7 +109,7 @@ module Gitlab length end - def longest_common_suffix(a, b) + def longest_common_suffix(a, b) # rubocop:disable Naming/UncommunicativeMethodParamName longest_common_prefix(a.reverse, b.reverse) end end diff --git a/lib/gitlab/encoding_helper.rb b/lib/gitlab/encoding_helper.rb index 5b5071ffc39..d1fd5dfe0cb 100644 --- a/lib/gitlab/encoding_helper.rb +++ b/lib/gitlab/encoding_helper.rb @@ -68,14 +68,14 @@ module Gitlab nil end - def encode_binary(s) - return "" if s.nil? + def encode_binary(str) + return "" if str.nil? - s.dup.force_encoding(Encoding::ASCII_8BIT) + str.dup.force_encoding(Encoding::ASCII_8BIT) end - def binary_stringio(s) - StringIO.new(s || '').tap { |io| io.set_encoding(Encoding::ASCII_8BIT) } + def binary_stringio(str) + StringIO.new(str || '').tap { |io| io.set_encoding(Encoding::ASCII_8BIT) } end private diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 8953bc8c148..a91de278cf3 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -191,19 +191,19 @@ module Gitlab end end - def linkify_issues(s) - s = s.gsub(/([Ii]ssue) ([0-9]+)/, '\1 #\2') - s = s.gsub(/([Cc]ase) ([0-9]+)/, '\1 #\2') - s + def linkify_issues(str) + str = str.gsub(/([Ii]ssue) ([0-9]+)/, '\1 #\2') + str = str.gsub(/([Cc]ase) ([0-9]+)/, '\1 #\2') + str end - def escape_for_markdown(s) - s = s.gsub(/^#/, "\\#") - s = s.gsub(/^-/, "\\-") - s = s.gsub("`", "\\~") - s = s.delete("\r") - s = s.gsub("\n", " \n") - s + def escape_for_markdown(str) + str = str.gsub(/^#/, "\\#") + str = str.gsub(/^-/, "\\-") + str = str.gsub("`", "\\~") + str = str.delete("\r") + str = str.gsub("\n", " \n") + str end def format_content(raw_content) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 66e781a8e5b..58a4060cc96 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -401,8 +401,8 @@ module Gitlab path.read.chomp end - def self.timestamp(t) - Google::Protobuf::Timestamp.new(seconds: t.to_i) + def self.timestamp(time) + Google::Protobuf::Timestamp.new(seconds: time.to_i) end # The default timeout on all Gitaly calls diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 72e1e59d8df..6a97cd8ed17 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -399,8 +399,8 @@ module Gitlab end end - def encode_repeated(a) - Google::Protobuf::RepeatedField.new(:bytes, a.map { |s| encode_binary(s) } ) + def encode_repeated(array) + Google::Protobuf::RepeatedField.new(:bytes, array.map { |s| encode_binary(s) } ) end def call_find_commit(revision) diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb index 02fcb413abd..8e530de174d 100644 --- a/lib/gitlab/gitaly_client/storage_settings.rb +++ b/lib/gitlab/gitaly_client/storage_settings.rb @@ -60,8 +60,8 @@ module Gitlab private - def method_missing(m, *args, &block) - @hash.public_send(m, *args, &block) # rubocop:disable GitlabSecurity/PublicSend + def method_missing(msg, *args, &block) + @hash.public_send(msg, *args, &block) # rubocop:disable GitlabSecurity/PublicSend end end end diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index 46b49128140..5070f4e3cfe 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -200,27 +200,27 @@ module Gitlab "Status: #{name}" end - def linkify_issues(s) - s = s.gsub(/([Ii]ssue) ([0-9]+)/, '\1 #\2') - s = s.gsub(/([Cc]omment) #([0-9]+)/, '\1 \2') - s + def linkify_issues(str) + str = str.gsub(/([Ii]ssue) ([0-9]+)/, '\1 #\2') + str = str.gsub(/([Cc]omment) #([0-9]+)/, '\1 \2') + str end - def escape_for_markdown(s) + def escape_for_markdown(str) # No headings and lists - s = s.gsub(/^#/, "\\#") - s = s.gsub(/^-/, "\\-") + str = str.gsub(/^#/, "\\#") + str = str.gsub(/^-/, "\\-") # No inline code - s = s.gsub("`", "\\`") + str = str.gsub("`", "\\`") # Carriage returns make me sad - s = s.delete("\r") + str = str.delete("\r") # Markdown ignores single newlines, but we need them as
. - s = s.gsub("\n", " \n") + str = str.gsub("\n", " \n") - s + str end def create_label(name) diff --git a/lib/rouge/formatters/html_gitlab.rb b/lib/rouge/formatters/html_gitlab.rb index be0d97370d0..e877ab10248 100644 --- a/lib/rouge/formatters/html_gitlab.rb +++ b/lib/rouge/formatters/html_gitlab.rb @@ -11,7 +11,7 @@ module Rouge @tag = tag end - def stream(tokens, &b) + def stream(tokens) is_first = true token_lines(tokens) do |line| yield "\n" unless is_first diff --git a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb index 6f729fec460..ba4c66057d4 100644 --- a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb @@ -175,7 +175,7 @@ describe MigrateGcpClustersToNewClustersArchitectures, :migration do end end - def tr(s) - s.delete("'") + def tr(str) + str.delete("'") end end diff --git a/spec/support/helpers/key_generator_helper.rb b/spec/support/helpers/key_generator_helper.rb index b1c289ffef7..d55d8312c65 100644 --- a/spec/support/helpers/key_generator_helper.rb +++ b/spec/support/helpers/key_generator_helper.rb @@ -24,7 +24,7 @@ module Spec private # Encodes an openssh-mpi-encoded integer. - def encode_mpi(n) + def encode_mpi(n) # rubocop:disable Naming/UncommunicativeMethodParamName chars, n = [], n.to_i chars << (n & 0xff) && n >>= 8 while n != 0 chars << 0 if chars.empty? || chars.last >= 0x80 diff --git a/spec/workers/concerns/waitable_worker_spec.rb b/spec/workers/concerns/waitable_worker_spec.rb index 199825b5097..ce38cde9208 100644 --- a/spec/workers/concerns/waitable_worker_spec.rb +++ b/spec/workers/concerns/waitable_worker_spec.rb @@ -18,8 +18,8 @@ describe WaitableWorker do def self.bulk_perform_inline(args_list) end - def perform(i = 0) - self.class.counter += i + def perform(count = 0) + self.class.counter += count end end end From 5cd7b8033a842e3cde19d3981a3cde3800fdd7f8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 4 Jul 2018 16:13:59 +0000 Subject: [PATCH 19/38] Use latest gitlab-styles --- .rubocop.yml | 6 ------ Gemfile.lock | 4 ++-- Gemfile.rails5.lock | 4 ++-- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d0584b46b06..0582bfe8d70 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -31,12 +31,6 @@ Style/MutableConstant: - 'ee/db/post_migrate/**/*' - 'ee/db/geo/migrate/**/*' -Naming/UncommunicativeMethodParamName: - Enabled: false - -Naming/MemoizedInstanceVariableName: - Enabled: false - Naming/FileName: ExpectMatchingDefinition: true Exclude: diff --git a/Gemfile.lock b/Gemfile.lock index ad0675ddb8f..7ca3d5f9eda 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -312,7 +312,7 @@ GEM mime-types (>= 1.16) posix-spawn (~> 0.3) gitlab-markup (1.6.4) - gitlab-styles (2.4.0) + gitlab-styles (2.4.1) rubocop (~> 0.54.0) rubocop-gitlab-security (~> 0.1.0) rubocop-rspec (~> 1.19) @@ -785,7 +785,7 @@ GEM unicode-display_width (~> 1.0, >= 1.0.1) rubocop-gitlab-security (0.1.1) rubocop (>= 0.51) - rubocop-rspec (1.22.1) + rubocop-rspec (1.22.2) rubocop (>= 0.52.1) ruby-enum (0.7.2) i18n diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index e167bbcb52e..1aeaef4202d 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -315,7 +315,7 @@ GEM mime-types (>= 1.16) posix-spawn (~> 0.3) gitlab-markup (1.6.4) - gitlab-styles (2.4.0) + gitlab-styles (2.4.1) rubocop (~> 0.54.0) rubocop-gitlab-security (~> 0.1.0) rubocop-rspec (~> 1.19) @@ -794,7 +794,7 @@ GEM unicode-display_width (~> 1.0, >= 1.0.1) rubocop-gitlab-security (0.1.1) rubocop (>= 0.51) - rubocop-rspec (1.22.1) + rubocop-rspec (1.22.2) rubocop (>= 0.52.1) ruby-enum (0.7.2) i18n From 72c99b58abe7bbc956804c9d403689e64f579d3e Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 9 Jul 2018 17:13:11 +0800 Subject: [PATCH 20/38] Remove useless return --- lib/gitlab/exclusive_lease_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/exclusive_lease_helpers.rb b/lib/gitlab/exclusive_lease_helpers.rb index ab6838adc6d..e998548cff9 100644 --- a/lib/gitlab/exclusive_lease_helpers.rb +++ b/lib/gitlab/exclusive_lease_helpers.rb @@ -21,7 +21,7 @@ module Gitlab raise FailedToObtainLockError, 'Failed to obtain a lock' unless uuid - return yield + yield ensure Gitlab::ExclusiveLease.cancel(key, uuid) end From 8186bb39031b189396b57def9de202ddf65cbcbb Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 9 Jul 2018 15:40:48 +0100 Subject: [PATCH 21/38] Removes unused store in diffs mr refactor Removes double export for actions in diffs module in mr refactor --- app/assets/javascripts/diffs/store/actions.js | 13 +-- app/assets/javascripts/diffs/store/index.js | 11 --- .../javascripts/diffs/store/modules/index.js | 2 +- changelogs/unreleased/48951-clean-up.yml | 5 + .../diffs/components/changed_files_spec.js | 97 ++++++++++--------- 5 files changed, 59 insertions(+), 69 deletions(-) delete mode 100644 app/assets/javascripts/diffs/store/index.js create mode 100644 changelogs/unreleased/48951-clean-up.yml diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 5e0fd5109bb..5bfe42618c2 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -82,14 +82,5 @@ export const expandAllFiles = ({ commit }) => { commit(types.EXPAND_ALL_FILES); }; -export default { - setBaseConfig, - fetchDiffFiles, - setInlineDiffViewType, - setParallelDiffViewType, - showCommentForm, - cancelCommentForm, - loadMoreLines, - loadCollapsedDiff, - expandAllFiles, -}; +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/app/assets/javascripts/diffs/store/index.js b/app/assets/javascripts/diffs/store/index.js deleted file mode 100644 index e6aa8f5b12a..00000000000 --- a/app/assets/javascripts/diffs/store/index.js +++ /dev/null @@ -1,11 +0,0 @@ -import Vue from 'vue'; -import Vuex from 'vuex'; -import diffsModule from './modules'; - -Vue.use(Vuex); - -export default new Vuex.Store({ - modules: { - diffs: diffsModule, - }, -}); diff --git a/app/assets/javascripts/diffs/store/modules/index.js b/app/assets/javascripts/diffs/store/modules/index.js index c745320d532..56c26d1ffad 100644 --- a/app/assets/javascripts/diffs/store/modules/index.js +++ b/app/assets/javascripts/diffs/store/modules/index.js @@ -1,4 +1,4 @@ -import actions from '../actions'; +import * as actions from '../actions'; import * as getters from '../getters'; import mutations from '../mutations'; import createState from './diff_state'; diff --git a/changelogs/unreleased/48951-clean-up.yml b/changelogs/unreleased/48951-clean-up.yml new file mode 100644 index 00000000000..0102cd43f96 --- /dev/null +++ b/changelogs/unreleased/48951-clean-up.yml @@ -0,0 +1,5 @@ +--- +title: Removes unused vuex code in mr refactor and removes unneeded dependencies +merge_request: 20499 +author: +type: other diff --git a/spec/javascripts/diffs/components/changed_files_spec.js b/spec/javascripts/diffs/components/changed_files_spec.js index 2d57af6137c..f737e8fa38e 100644 --- a/spec/javascripts/diffs/components/changed_files_spec.js +++ b/spec/javascripts/diffs/components/changed_files_spec.js @@ -1,12 +1,17 @@ import Vue from 'vue'; -import $ from 'jquery'; +import Vuex from 'vuex'; import { mountComponentWithStore } from 'spec/helpers'; -import store from '~/diffs/store'; -import ChangedFiles from '~/diffs/components/changed_files.vue'; +import diffsModule from '~/diffs/store/modules'; +import changedFiles from '~/diffs/components/changed_files.vue'; describe('ChangedFiles', () => { - const Component = Vue.extend(ChangedFiles); - const createComponent = props => mountComponentWithStore(Component, { props, store }); + const Component = Vue.extend(changedFiles); + const store = new Vuex.Store({ + modules: { + diffs: diffsModule, + }, + }); + let vm; beforeEach(() => { @@ -14,6 +19,7 @@ describe('ChangedFiles', () => {
`); + const props = { diffFiles: [ { @@ -26,7 +32,8 @@ describe('ChangedFiles', () => { }, ], }; - vm = createComponent(props); + + vm = mountComponentWithStore(Component, { props, store }); }); describe('with single file added', () => { @@ -40,58 +47,56 @@ describe('ChangedFiles', () => { }); }); - describe('template', () => { - describe('diff view mode buttons', () => { - let inlineButton; - let parallelButton; + describe('diff view mode buttons', () => { + let inlineButton; + let parallelButton; - beforeEach(() => { - inlineButton = vm.$el.querySelector('.js-inline-diff-button'); - parallelButton = vm.$el.querySelector('.js-parallel-diff-button'); + beforeEach(() => { + inlineButton = vm.$el.querySelector('.js-inline-diff-button'); + parallelButton = vm.$el.querySelector('.js-parallel-diff-button'); + }); + + it('should have Inline and Side-by-side buttons', () => { + expect(inlineButton).toBeDefined(); + expect(parallelButton).toBeDefined(); + }); + + it('should add active class to Inline button', done => { + vm.$store.state.diffs.diffViewType = 'inline'; + + vm.$nextTick(() => { + expect(inlineButton.classList.contains('active')).toEqual(true); + expect(parallelButton.classList.contains('active')).toEqual(false); + + done(); }); + }); - it('should have Inline and Side-by-side buttons', () => { - expect(inlineButton).toBeDefined(); - expect(parallelButton).toBeDefined(); + it('should toggle active state of buttons when diff view type changed', done => { + vm.$store.state.diffs.diffViewType = 'parallel'; + + vm.$nextTick(() => { + expect(inlineButton.classList.contains('active')).toEqual(false); + expect(parallelButton.classList.contains('active')).toEqual(true); + + done(); }); + }); - it('should add active class to Inline button', done => { - vm.$store.state.diffs.diffViewType = 'inline'; - - vm.$nextTick(() => { - expect(inlineButton.classList.contains('active')).toEqual(true); - expect(parallelButton.classList.contains('active')).toEqual(false); - - done(); - }); - }); - - it('should toggle active state of buttons when diff view type changed', done => { - vm.$store.state.diffs.diffViewType = 'parallel'; + describe('clicking them', () => { + it('should toggle the diff view type', done => { + parallelButton.click(); vm.$nextTick(() => { expect(inlineButton.classList.contains('active')).toEqual(false); expect(parallelButton.classList.contains('active')).toEqual(true); - done(); - }); - }); - - describe('clicking them', () => { - it('should toggle the diff view type', done => { - $(parallelButton).click(); + inlineButton.click(); vm.$nextTick(() => { - expect(inlineButton.classList.contains('active')).toEqual(false); - expect(parallelButton.classList.contains('active')).toEqual(true); - - $(inlineButton).click(); - - vm.$nextTick(() => { - expect(inlineButton.classList.contains('active')).toEqual(true); - expect(parallelButton.classList.contains('active')).toEqual(false); - done(); - }); + expect(inlineButton.classList.contains('active')).toEqual(true); + expect(parallelButton.classList.contains('active')).toEqual(false); + done(); }); }); }); From d8013704b66a9f8653af87c8202d980cc30cb003 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Mon, 9 Jul 2018 14:53:33 +0000 Subject: [PATCH 22/38] Document that we don't want to wait in tests --- doc/development/testing_guide/frontend_testing.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 3b2b9c8c947..f8993653aec 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -172,6 +172,10 @@ object which can be treated like any other jasmine spy object. Further documentation on the babel rewire pluign API can be found on [its repository Readme doc](https://github.com/speedskater/babel-plugin-rewire#babel-plugin-rewire). +#### Waiting in tests + +If you cannot avoid using [`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout) in tests, please use the [Jasmine mock clock](https://jasmine.github.io/api/2.9/Clock.html). + ### Vue.js unit tests See this [section][vue-test]. From 47f5dab6923b438c3a1e27671bd44333971191b9 Mon Sep 17 00:00:00 2001 From: Marcia Ramos Date: Mon, 9 Jul 2018 15:17:10 +0000 Subject: [PATCH 23/38] Docs: make it clear that you need a completely separate domain for Pages --- doc/administration/pages/index.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index 9b1297ca4ba..c0a281382a5 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -49,8 +49,8 @@ supporting custom domains a secondary IP is not needed. Before proceeding with the Pages configuration, you will need to: -1. Have a separate domain under which the GitLab Pages will be served. In this - document we assume that to be `example.io`. +1. Have an exclusive root domain for serving GitLab Pages. Note that you cannot + use a subdomain of your GitLab's instance domain. 1. Configure a **wildcard DNS record**. 1. (Optional) Have a **wildcard certificate** for that domain if you decide to serve Pages under HTTPS. From e6f3452314c73875bbf9560a84706161c25c2831 Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Mon, 9 Jul 2018 13:24:29 +0100 Subject: [PATCH 24/38] Adds with_projects optional parameter to /groups/:id API endpoint --- ...-omit-projects-from-get-group-endpoint.yml | 5 ++++ doc/api/groups.md | 25 +++++++++++++++++ lib/api/groups.rb | 3 ++- spec/requests/api/groups_spec.rb | 27 ++++++++++++++++--- 4 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/42415-omit-projects-from-get-group-endpoint.yml diff --git a/changelogs/unreleased/42415-omit-projects-from-get-group-endpoint.yml b/changelogs/unreleased/42415-omit-projects-from-get-group-endpoint.yml new file mode 100644 index 00000000000..cabe5216045 --- /dev/null +++ b/changelogs/unreleased/42415-omit-projects-from-get-group-endpoint.yml @@ -0,0 +1,5 @@ +--- +title: Adds with_projects optional parameter to GET /groups/:id API endpoint +merge_request: 20494 +author: +type: changed diff --git a/doc/api/groups.md b/doc/api/groups.md index 53d72509423..11de75039ee 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -210,6 +210,7 @@ Parameters: | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user | | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | +| `with_projects` | boolean | no | Include details from projects that belong to the specified group (defaults to `true`). | ```bash curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/4 @@ -361,6 +362,30 @@ Example response: } ``` +When adding the parameter `with_projects=false`, projects will not be returned. + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/4?with_projects=false +``` + +Example response: + +```json +{ + "id": 4, + "name": "Twitter", + "path": "twitter", + "description": "Aliquid qui quis dignissimos distinctio ut commodi voluptas est.", + "visibility": "public", + "avatar_url": null, + "web_url": "https://gitlab.example.com/groups/twitter", + "request_access_enabled": false, + "full_name": "Twitter", + "full_path": "twitter", + "parent_id": null +} +``` + ## New group Creates a new project group. Available only for users who can create groups. diff --git a/lib/api/groups.rb b/lib/api/groups.rb index f633dd88d06..797b04df059 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -150,12 +150,13 @@ module API end params do use :with_custom_attributes + optional :with_projects, type: Boolean, default: true, desc: 'Omit project details' end get ":id" do group = find_group!(params[:id]) options = { - with: Entities::GroupDetail, + with: params[:with_projects] ? Entities::GroupDetail : Entities::Group, current_user: current_user } diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index da23fdd7dca..5a04e699d60 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -251,14 +251,22 @@ describe API::Groups do projects end + def response_project_ids(json_response, key) + json_response[key].map do |project| + project['id'].to_i + end + end + context 'when unauthenticated' do it 'returns 404 for a private group' do get api("/groups/#{group2.id}") + expect(response).to have_gitlab_http_status(404) end it 'returns 200 for a public group' do get api("/groups/#{group1.id}") + expect(response).to have_gitlab_http_status(200) end @@ -268,7 +276,7 @@ describe API::Groups do get api("/groups/#{public_group.id}") - expect(json_response['projects'].map { |p| p['id'].to_i }) + expect(response_project_ids(json_response, 'projects')) .to contain_exactly(projects[:public].id) end @@ -278,7 +286,7 @@ describe API::Groups do get api("/groups/#{group1.id}") - expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + expect(response_project_ids(json_response, 'shared_projects')) .to contain_exactly(projects[:public].id) end end @@ -309,6 +317,17 @@ describe API::Groups do expect(json_response['shared_projects'][0]['id']).to eq(project.id) end + it "returns one of user1's groups without projects when with_projects option is set to false" do + project = create(:project, namespace: group2, path: 'Foo') + create(:project_group_link, project: project, group: group1) + + get api("/groups/#{group1.id}", user1), with_projects: false + + expect(response).to have_gitlab_http_status(200) + expect(json_response['projects']).to be_nil + expect(json_response['shared_projects']).to be_nil + end + it "does not return a non existing group" do get api("/groups/1328", user1) @@ -327,7 +346,7 @@ describe API::Groups do get api("/groups/#{public_group.id}", user2) - expect(json_response['projects'].map { |p| p['id'].to_i }) + expect(response_project_ids(json_response, 'projects')) .to contain_exactly(projects[:public].id, projects[:internal].id) end @@ -337,7 +356,7 @@ describe API::Groups do get api("/groups/#{group1.id}", user2) - expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + expect(response_project_ids(json_response, 'shared_projects')) .to contain_exactly(projects[:public].id, projects[:internal].id) end end From 75e1f256385ed16b6a0b5bc40e52ad169d041b41 Mon Sep 17 00:00:00 2001 From: Kaspar Emanuel Date: Sun, 8 Jul 2018 22:09:44 +0100 Subject: [PATCH 25/38] Fix API docs on unauthenticated projects return --- doc/api/projects.md | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/doc/api/projects.md b/doc/api/projects.md index 1e06f6d01f3..a35c2a56992 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -34,7 +34,7 @@ There are currently three options for `merge_method` to choose from: ## List all projects Get a list of all visible projects across GitLab for the authenticated user. -When accessed without authentication, only public projects are returned. +When accessed without authentication, only public projects with "simple" fields are returned. ``` GET /projects @@ -47,7 +47,7 @@ GET /projects | `order_by` | string | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` | | `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Return list of projects matching the search criteria | -| `simple` | boolean | no | Return only the ID, URL, name, and path of each project | +| `simple` | boolean | no | Return only limited fields for each project. This is a no-op without authentication as then _only_ simple fields are returned. | | `owned` | boolean | no | Limit by projects owned by the current user | | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | @@ -56,6 +56,41 @@ GET /projects | `with_issues_enabled` | boolean | no | Limit by enabled issues feature | | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature | +When `simple=true` or the user is unauthenticated this returns something like: + +```json +[ + { + "id": 4, + "description": null, + "default_branch": "master", + "ssh_url_to_repo": "git@example.com:diaspora/diaspora-client.git", + "http_url_to_repo": "http://example.com/diaspora/diaspora-client.git", + "web_url": "http://example.com/diaspora/diaspora-client", + "readme_url": "http://example.com/diaspora/diaspora-client/blob/master/README.md", + "tag_list": [ + "example", + "disapora client" + ], + "name": "Diaspora Client", + "name_with_namespace": "Diaspora / Diaspora Client", + "path": "diaspora-client", + "path_with_namespace": "diaspora/diaspora-client", + "created_at": "2013-09-30T13:46:02Z", + "last_activity_at": "2013-09-30T13:46:02Z", + "forks_count": 0, + "avatar_url": "http://example.com/uploads/project/avatar/4/uploads/avatar.png", + "star_count": 0, + }, + { + "id": 6, + "description": null, + "default_branch": "master", +... +``` + +When the user is authenticated and `simple` is not set this returns something like: + ```json [ { @@ -235,7 +270,7 @@ GET /users/:user_id/projects | `order_by` | string | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` | | `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Return list of projects matching the search criteria | -| `simple` | boolean | no | Return only the ID, URL, name, and path of each project | +| `simple` | boolean | no | Return only limited fields for each project. This is a no-op without authentication as then _only_ simple fields are returned. | | `owned` | boolean | no | Limit by projects owned by the current user | | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | @@ -723,7 +758,7 @@ GET /projects/:id/forks | `order_by` | string | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` | | `sort` | string | no | Return projects sorted in `asc` or `desc` order. Default is `desc` | | `search` | string | no | Return list of projects matching the search criteria | -| `simple` | boolean | no | Return only the ID, URL, name, and path of each project | +| `simple` | boolean | no | Return only limited fields for each project. This is a no-op without authentication as then _only_ simple fields are returned. | | `owned` | boolean | no | Limit by projects owned by the current user | | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | From 5e8ff175d5c3b8e1d45704950ac47777946a7e47 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 9 Jul 2018 16:04:22 +0000 Subject: [PATCH 26/38] Fix search bar text input alignment --- app/assets/stylesheets/framework/filters.scss | 5 ++++- changelogs/unreleased/fix-search-bar.yml | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/fix-search-bar.yml diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 551a7e852ae..5d79610b21e 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -224,7 +224,10 @@ .form-control { position: relative; min-width: 200px; - padding: 5px 25px 6px 0; + padding-right: 25px; + padding-left: 0; + height: $input-height; + line-height: inherit; border-color: transparent; &:focus, diff --git a/changelogs/unreleased/fix-search-bar.yml b/changelogs/unreleased/fix-search-bar.yml new file mode 100644 index 00000000000..d4c0c1efddf --- /dev/null +++ b/changelogs/unreleased/fix-search-bar.yml @@ -0,0 +1,5 @@ +--- +title: Fix search bar text input alignment +merge_request: +author: +type: fixed From 737666a3d121b1bf89861de4445f857256a47949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Mon, 9 Jul 2018 18:13:34 +0200 Subject: [PATCH 27/38] Fix specs --- app/uploaders/gitlab_uploader.rb | 11 ++++++----- spec/controllers/projects/jobs_controller_spec.rb | 11 +++++------ spec/requests/api/jobs_spec.rb | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 7aa81a8c312..719bd6ef418 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -76,11 +76,12 @@ class GitlabUploader < CarrierWave::Uploader::Base end def open - stream = if file_storage? - File.open(path, "rb") if path - else - ::Gitlab::HttpIO.new(url, cached_size) if url - end + stream = + if file_storage? + File.open(path, "rb") if path + else + ::Gitlab::HttpIO.new(url, cached_size) if url + end return unless stream return stream unless block_given? diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 38431fb1598..6be27126383 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -216,20 +216,19 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do end context 'when trace artifact is in ObjectStorage' do + let(:url) { 'http://object-storage/trace' } + let(:file_path) { expand_fixture_path('trace/sample_trace') } let!(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) } before do allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false } - allow_any_instance_of(JobArtifactUploader).to receive(:url) { remote_trace_url } - allow_any_instance_of(JobArtifactUploader).to receive(:size) { remote_trace_size } + allow_any_instance_of(JobArtifactUploader).to receive(:url) { url } + allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file_path) } end context 'when there are no network issues' do - let(:url) { 'http://object-storage/trace' } - let(:file) { expand_fixture_path('trace/sample_trace') } - before do - stub_remote_url_206(url, file) + stub_remote_url_206(url, file_path) get_trace end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 8a2812d1576..0609166ed90 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -536,13 +536,13 @@ describe API::Jobs do context 'when trace is in ObjectStorage' do let!(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) } let(:url) { 'http://object-storage/trace' } - let(:file) { expand_fixture_path('trace/sample_trace') } + let(:file_path) { expand_fixture_path('trace/sample_trace') } before do stub_remote_url_206(url, file_path) allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false } allow_any_instance_of(JobArtifactUploader).to receive(:url) { url } - allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file) } + allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file_path) } end it 'returns specific job trace' do From 963cd5a5b3cf6ab76b99bc5aa33622c217face1b Mon Sep 17 00:00:00 2001 From: Yi-Jyun Pan Date: Mon, 9 Jul 2018 16:15:43 +0000 Subject: [PATCH 28/38] Request to be a proofreader. --- doc/development/i18n/proofreader.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index 9d0d7348df9..ca8ebcdc984 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -11,6 +11,7 @@ are very appreciative of the work done by translators and proofreaders! - Chinese Traditional - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) - Weizhe Ding - [GitLab](https://gitlab.com/d.weizhe), [Crowdin](https://crowdin.com/profile/d.weizhe) + - Yi-Jyun Pan - [GitLab](https://gitlab.com/pan93412), [Crowdin](https://crowdin.com/profile/pan93412) - Chinese Traditional, Hong Kong - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) - Dutch From e6a49b0076dbc945230d67571489fc8782811fab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20K=C3=A4mmerle?= Date: Mon, 9 Jul 2018 16:34:50 +0000 Subject: [PATCH 29/38] Add additional headline for Adding SSH keys to GitLab --- doc/ssh/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/ssh/README.md b/doc/ssh/README.md index bab196e7609..63f0a654fcf 100644 --- a/doc/ssh/README.md +++ b/doc/ssh/README.md @@ -77,6 +77,8 @@ Note that Public SSH key may also be named as follows: If you want to change the password of your SSH key pair, you can use `ssh-keygen -p `. +## Adding a SSH key to your GitLab account + 1. The next step is to copy the public SSH key as we will need it afterwards. To copy your public SSH key to the clipboard, use the appropriate code below: From 38d407d7a7fd07888cbfda26360cd0a1c4972ec5 Mon Sep 17 00:00:00 2001 From: Jamie Schembri Date: Mon, 9 Jul 2018 17:44:09 +0200 Subject: [PATCH 30/38] Fix #48537 - Update avatar only via the projects API --- .../48537-update-avatar-only-via-api.yml | 5 +++++ lib/api/projects.rb | 3 ++- spec/requests/api/projects_spec.rb | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/48537-update-avatar-only-via-api.yml diff --git a/changelogs/unreleased/48537-update-avatar-only-via-api.yml b/changelogs/unreleased/48537-update-avatar-only-via-api.yml new file mode 100644 index 00000000000..9b3ab946cc1 --- /dev/null +++ b/changelogs/unreleased/48537-update-avatar-only-via-api.yml @@ -0,0 +1,5 @@ +--- +title: Allow updating a project's avatar without other params +merge_request: +author: Jamie Schembri +type: fixed diff --git a/lib/api/projects.rb b/lib/api/projects.rb index b83da00502d..8273abe48c9 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -260,7 +260,8 @@ module API :snippets_enabled, :tag_list, :visibility, - :wiki_enabled + :wiki_enabled, + :avatar ] optional :name, type: String, desc: 'The name of the project' optional :default_branch, type: String, desc: 'The default branch of the project' diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index de540ba7a10..15da81b57db 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1526,6 +1526,20 @@ describe API::Projects do expect(response).to have_gitlab_http_status(400) end + + it 'updates avatar' do + project_param = { + avatar: fixture_file_upload('spec/fixtures/banana_sample.gif', + 'image/gif') + } + + put api("/projects/#{project3.id}", user), project_param + + expect(response).to have_gitlab_http_status(200) + expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ + '-/system/project/avatar/'\ + "#{project3.id}/banana_sample.gif") + end end context 'when authenticated as project master' do From ded205b4024109942dc50594f504f0e85e2447bd Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Mon, 9 Jul 2018 22:41:19 +0200 Subject: [PATCH 31/38] Fix mountComponent helper path in docs --- doc/development/fe_guide/vue.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index e31ee087358..219b98ac696 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -425,7 +425,7 @@ There is a helper in `spec/javascripts/helpers/vue_mount_component_helper.js` th ```javascript import Vue from 'vue'; -import mountComponent from 'helpers/vue_mount_component_helper.js' +import mountComponent from 'spec/helpers/vue_mount_component_helper' import component from 'component.vue' const Component = Vue.extend(component); From 0198057b1462b55beede9d604e54249f4ee63145 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 9 Jul 2018 13:54:34 -0700 Subject: [PATCH 33/38] Shorten CHANGELOG filename to avoid breaking ecryptfs users [ci skip] Closes #49038 --- ...links-namespace.yml => 48976-fix-sti-background-migration.yml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelogs/unreleased/{48976-rails5-invalid-single-table-inheritance-type-group-is-not-a-subclass-of-gitlab-backgroundmigration-fixcrossprojectlabellinks-namespace.yml => 48976-fix-sti-background-migration.yml} (100%) diff --git a/changelogs/unreleased/48976-rails5-invalid-single-table-inheritance-type-group-is-not-a-subclass-of-gitlab-backgroundmigration-fixcrossprojectlabellinks-namespace.yml b/changelogs/unreleased/48976-fix-sti-background-migration.yml similarity index 100% rename from changelogs/unreleased/48976-rails5-invalid-single-table-inheritance-type-group-is-not-a-subclass-of-gitlab-backgroundmigration-fixcrossprojectlabellinks-namespace.yml rename to changelogs/unreleased/48976-fix-sti-background-migration.yml From 6b733bc45753cdaef6ac71941e1abc2fbf1af26d Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Mon, 9 Jul 2018 21:35:37 +0000 Subject: [PATCH 34/38] Fix link to frontend in handbook --- doc/development/fe_guide/development_process.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md index d240dbe8b02..905668eef58 100644 --- a/doc/development/fe_guide/development_process.md +++ b/doc/development/fe_guide/development_process.md @@ -1,6 +1,6 @@ # Frontend Development Process -You can find more about the organization of the frontend team in the [handbook](https://about.gitlab.com/handbook/frontend/). +You can find more about the organization of the frontend team in the [handbook](https://about.gitlab.com/handbook/engineering/frontend/). ## Development Checklist @@ -34,7 +34,7 @@ Please use your best judgement when to use it and please contribute new points t - [ ] **Cookie Mode** Think about hiding the feature behind a cookie flag if the implementation is on top of existing features - [ ] **New route** Are you refactoring something big then you might consider adding a new route where you implement the new feature and when finished delete the current route and rename the new one. (for example 'merge_request' and 'new_merge_request') - [ ] **Setup** Is there any specific setup needed for your implementation (for example a kubernetes cluster)? Then let everyone know if it is not already mentioned where they can find documentation (if it doesn't exist - create it) -- [ ] **Security** Are there any new security relevant implementations? Then please contact the security team for an app security review. If you are not sure ask our [domain expert](https://about.gitlab.com/handbook/frontend/#frontend-domain-experts) +- [ ] **Security** Are there any new security relevant implementations? Then please contact the security team for an app security review. If you are not sure ask our [domain expert](https://about.gitlab.com/handbook/engineering/frontend/#frontend-domain-experts) #### During development @@ -51,7 +51,7 @@ Please use your best judgement when to use it and please contribute new points t - [ ] **Performance** Have you checked performance? For example do the same thing with 500 comments instead of 1. Document the tests and possible findings in the MR so a reviewer can directly see it. - [ ] Have you tested with a variety of our [supported browsers](../../install/requirements.md#supported-web-browsers)? You can use [browserstack](https://www.browserstack.com/) to be able to access a wide variety of browsers and operating systems. - [ ] Did you check the mobile view? -- [ ] Check the built webpack bundle (For the report run `WEBPACK_REPORT=true gdk run`, then open `webpack-report/index.html`) if we have unnecessary bloat due to wrong references, including libraries multiple times, etc.. If you need help contact the webpack [domain expert](https://about.gitlab.com/handbook/frontend/#frontend-domain-experts) +- [ ] Check the built webpack bundle (For the report run `WEBPACK_REPORT=true gdk run`, then open `webpack-report/index.html`) if we have unnecessary bloat due to wrong references, including libraries multiple times, etc.. If you need help contact the webpack [domain expert](https://about.gitlab.com/handbook/engineering/frontend/#frontend-domain-experts) - [ ] **Tests** Not only greenfield tests - Test also all bad cases that come to your mind. - [ ] If you have multiple MR's then also smoke test against the final merge. - [ ] Are there any big changes on how and especially how frequently we use the API then let production know about it From d79cef3a9a5577765d975326fbf4bc1b8c5634de Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Tue, 10 Jul 2018 08:11:04 +0000 Subject: [PATCH 35/38] Support manually stopping any environment from the UI --- .../components/environment_actions.vue | 91 +- .../components/environment_external_url.vue | 51 +- .../components/environment_item.vue | 867 +++++++++--------- .../components/environment_monitoring.vue | 49 +- .../components/environment_rollback.vue | 96 +- .../components/environment_stop.vue | 114 +-- .../environment_terminal_button.vue | 53 +- .../components/environments_app.vue | 4 + .../components/stop_environment_modal.vue | 92 ++ .../folder/environments_folder_view.vue | 8 + .../environments/mixins/environments_mixin.js | 22 +- .../services/environments_service.js | 2 +- .../stylesheets/pages/environments.scss | 2 +- .../projects/environments_controller.rb | 6 +- .../projects/merge_requests_controller.rb | 2 +- app/policies/environment_policy.rb | 10 +- app/serializers/environment_entity.rb | 12 +- app/views/projects/deployments/_actions.haml | 7 +- app/views/projects/deployments/_rollback.haml | 7 +- .../environments/_external_url.html.haml | 2 +- .../projects/environments/_stop.html.haml | 5 - .../projects/environments/show.html.haml | 32 +- .../unreleased/winh-stop-all-environments.yml | 5 + lib/api/environments.rb | 3 +- locale/gitlab.pot | 37 +- .../projects/environments/environment_spec.rb | 13 +- .../environments/environments_spec.rb | 12 +- .../environments/environment_rollback_spec.js | 4 +- .../environments/environment_stop_spec.js | 12 +- spec/policies/environment_policy_spec.rb | 128 ++- spec/serializers/environment_entity_spec.rb | 3 +- .../environment_serializer_spec.rb | 10 +- 32 files changed, 1009 insertions(+), 752 deletions(-) create mode 100644 app/assets/javascripts/environments/components/stop_environment_modal.vue delete mode 100644 app/views/projects/environments/_stop.html.haml create mode 100644 changelogs/unreleased/winh-stop-all-environments.yml diff --git a/app/assets/javascripts/environments/components/environment_actions.vue b/app/assets/javascripts/environments/components/environment_actions.vue index e3652fe739e..63d83e307ee 100644 --- a/app/assets/javascripts/environments/components/environment_actions.vue +++ b/app/assets/javascripts/environments/components/environment_actions.vue @@ -1,50 +1,50 @@