From b20405ebc2352fe0e460d35bee7dffc334b13501 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Mon, 8 Jul 2019 16:39:53 +1200 Subject: Add documentation for Design Management feature https://gitlab.com/gitlab-org/gitlab-ce/issues/64243 --- doc/user/permissions.md | 2 + doc/user/project/issues/design_management.md | 58 +++++++++++++++++++++ .../project/issues/img/design_management_v12_2.png | Bin 0 -> 344504 bytes doc/user/project/issues/index.md | 6 +++ 4 files changed, 66 insertions(+) create mode 100644 doc/user/project/issues/design_management.md create mode 100644 doc/user/project/issues/img/design_management_v12_2.png diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 619cf34b5c3..238f95378b2 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -47,6 +47,7 @@ The following table depicts the various user permission levels in a project. | View approved/blacklisted licenses **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | | View license management reports **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View Security reports **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | +| View [Design Management](project/issues/design_management.md) pages **(PREMIUM)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View project code | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | Pull project code | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View GitLab Pages protected by [access control](project/pages/introduction.md#gitlab-pages-access-control-core-only) | ✓ | ✓ | ✓ | ✓ | ✓ | @@ -74,6 +75,7 @@ The following table depicts the various user permission levels in a project. | View Error Tracking list | | ✓ | ✓ | ✓ | ✓ | | Pull from [Maven repository](project/packages/maven_repository.md) or [NPM registry](project/packages/npm_registry.md) **(PREMIUM)** | | ✓ | ✓ | ✓ | ✓ | | Publish to [Maven repository](project/packages/maven_repository.md) or [NPM registry](project/packages/npm_registry.md) **(PREMIUM)** | | | ✓ | ✓ | ✓ || +| Upload [Design Management](project/issues/design_management.md) files **(PREMIUM)** | | | ✓ | ✓ | ✓ | | Create new branches | | | ✓ | ✓ | ✓ | | Push to non-protected branches | | | ✓ | ✓ | ✓ | | Force push to non-protected branches | | | ✓ | ✓ | ✓ | diff --git a/doc/user/project/issues/design_management.md b/doc/user/project/issues/design_management.md new file mode 100644 index 00000000000..2327fa84998 --- /dev/null +++ b/doc/user/project/issues/design_management.md @@ -0,0 +1,58 @@ +# Design Management **(PREMIUM)** + +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/660) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.2. + +CAUTION: **Warning:** +This an __alpha__ feature and is subject to change at any time without +prior notice. + +## Overview + +Design Management allows you to upload design assets (wireframes, mockups, etc.) +to GitLab issues and keep them stored in one single place, accessed by the Design +Management's page within an issue, giving product designers, product managers, and engineers a +way to collaborate on designs over one single source of truth. + +You can easily share mock-ups of designs with your team, or visual regressions can be easily +viewed and addressed. + + +For an overview, see the video [Design Management (GitLab 12.2)](https://www.youtube.com/watch?v=CCMtCqdK_aM). + +## Requirements + +Design Management requires +[Large File Storage (LFS)](../../../workflow/lfs/manage_large_binaries_with_git_lfs.md) +to be enabled: + +- For GitLab.com, LFS is already enabled. +- For self-managed instances, a GitLab administrator must have + [enabled LFS globally](../../../workflow/lfs/lfs_administration.md). +- For both GitLab.com and self-managed instances: LFS must be enabled for the project itself. + If enabled globally, LFS will be enabled by default to all projects. To enable LFS on the + project level, navigate to your project's **Settings > General**, expand **Visibility, project features, permissions** + and enable **Git Large File Storage**. + +## Limitations + +- Files uploaded must have a file extension of either `png`, `jpg`, `jpeg`, `gif`, `bmp`, `tiff` or `ico`. The [`svg` extension is not yet supported](https://gitlab.com/gitlab-org/gitlab-ee/issues/12771). +- [Designs cannot yet be deleted](https://gitlab.com/gitlab-org/gitlab-ee/issues/11089). +- Design Management is [not yet supported in the project export](https://gitlab.com/gitlab-org/gitlab-ee/issues/11090). + +## The Design Management page + +Navigate to the **Design Management** page from any issue by clicking the **Designs** tab: + +![Designs tab](img/design_management_v12_2.png) + +## Adding designs + +To upload design images, click the **Upload Designs** button and select images to upload. + +Designs with the same filename as an existing uploaded design will create a new version +of the design, and will replace the previous version. + +## Viewing designs + +Images on the Design Management page can be enlarged by clicking on them. + diff --git a/doc/user/project/issues/img/design_management_v12_2.png b/doc/user/project/issues/img/design_management_v12_2.png new file mode 100644 index 00000000000..6da747a3f21 Binary files /dev/null and b/doc/user/project/issues/img/design_management_v12_2.png differ diff --git a/doc/user/project/issues/index.md b/doc/user/project/issues/index.md index e917697e973..3decea9ceaa 100644 --- a/doc/user/project/issues/index.md +++ b/doc/user/project/issues/index.md @@ -119,6 +119,12 @@ associated label or assignee will change to match that of the new column. The en board can also be filtered to only include issues from a certain milestone or an overarching label. +### Design Management **(PREMIUM)** + +With [Design Management](design_management.md), you can upload design +assets to issues and view them all together to easily share and +collaborate with your team. + ### Epics **(ULTIMATE)** [Epics](../../group/epics/index.md) let you manage your portfolio of projects more -- cgit v1.2.1 From a7fa9bc248bda1044113e33f9ff0eca8938af0b2 Mon Sep 17 00:00:00 2001 From: Rayana Verissimo Date: Sat, 20 Jul 2019 17:50:35 +0000 Subject: Update tooltip values to meet design specs --- app/assets/stylesheets/framework/tooltips.scss | 5 ++--- app/assets/stylesheets/framework/variables_overrides.scss | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/framework/tooltips.scss b/app/assets/stylesheets/framework/tooltips.scss index 98f28987a82..edc2fb532c8 100644 --- a/app/assets/stylesheets/framework/tooltips.scss +++ b/app/assets/stylesheets/framework/tooltips.scss @@ -1,7 +1,6 @@ .tooltip-inner { - font-size: $tooltip-font-size; + font-size: $gl-font-size-small; border-radius: $border-radius-default; - line-height: 16px; + line-height: $gl-line-height; font-weight: $gl-font-weight-normal; - padding: 8px; } diff --git a/app/assets/stylesheets/framework/variables_overrides.scss b/app/assets/stylesheets/framework/variables_overrides.scss index ea96381a098..604b48e11ab 100644 --- a/app/assets/stylesheets/framework/variables_overrides.scss +++ b/app/assets/stylesheets/framework/variables_overrides.scss @@ -48,3 +48,7 @@ $spacers: ( 9: ($spacer * 8) ); $pagination-color: $gl-text-color; +$tooltip-padding-y: 0.5rem; +$tooltip-padding-x: 0.75rem; +$tooltip-arrow-height: 0.5rem; +$tooltip-arrow-width: 1rem; -- cgit v1.2.1 From f6bfd51deaf1b13f850536be7b2ddee17b1e3a7d Mon Sep 17 00:00:00 2001 From: Tiger Date: Mon, 22 Jul 2019 15:44:28 +1000 Subject: Remove ignore rule for ProjectAutoDevops#domain --- app/models/project_auto_devops.rb | 4 ---- spec/models/project_auto_devops_spec.rb | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index f39f54f0434..e11d0c48b4b 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true class ProjectAutoDevops < ApplicationRecord - include IgnorableColumn - - ignore_column :domain - belongs_to :project, inverse_of: :auto_devops enum deploy_strategy: { diff --git a/spec/models/project_auto_devops_spec.rb b/spec/models/project_auto_devops_spec.rb index 7bdd2367a68..da9e56ef897 100644 --- a/spec/models/project_auto_devops_spec.rb +++ b/spec/models/project_auto_devops_spec.rb @@ -15,7 +15,7 @@ describe ProjectAutoDevops do it { is_expected.to respond_to(:updated_at) } describe '#predefined_variables' do - let(:auto_devops) { build_stubbed(:project_auto_devops, project: project, domain: domain) } + let(:auto_devops) { build_stubbed(:project_auto_devops, project: project) } context 'when deploy_strategy is manual' do let(:auto_devops) { build_stubbed(:project_auto_devops, :manual_deployment, project: project) } -- cgit v1.2.1 From 657a5cc9d5df9eb4607d360b57d499c896fe1f73 Mon Sep 17 00:00:00 2001 From: Sanad Liaquat Date: Mon, 22 Jul 2019 15:58:43 +0500 Subject: Adds visibility attr to project --- qa/qa/page/project/new.rb | 2 +- qa/qa/resource/project.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/qa/qa/page/project/new.rb b/qa/qa/page/project/new.rb index 0918445d119..64aab9be056 100644 --- a/qa/qa/page/project/new.rb +++ b/qa/qa/page/project/new.rb @@ -59,7 +59,7 @@ module QA end def set_visibility(visibility) - choose visibility + choose visibility.capitalize end def click_github_link diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index c0a6004fe27..93a82094776 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -8,6 +8,7 @@ module QA include Events::Project attr_writer :initialize_with_readme + attr_writer :visibility attribute :id attribute :name @@ -44,6 +45,7 @@ module QA @standalone = false @description = 'My awesome project' @initialize_with_readme = false + @visibility = 'public' end def name=(raw_name) @@ -60,7 +62,7 @@ module QA page.choose_test_namespace page.choose_name(@name) page.add_description(@description) - page.set_visibility('Public') + page.set_visibility(@visibility) page.enable_initialize_with_readme if @initialize_with_readme page.create_new_project end @@ -88,7 +90,7 @@ module QA post_body = { name: name, description: description, - visibility: 'public', + visibility: @visibility, initialize_with_readme: @initialize_with_readme } -- cgit v1.2.1 From be7ad341467ac101ace61e6c56df8abe84e4e481 Mon Sep 17 00:00:00 2001 From: Jeffrey Cafferata Date: Mon, 22 Jul 2019 13:41:02 +0200 Subject: Removed the unnecessary loop through `../project_services/slack.md`. --- doc/integration/slack.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/integration/slack.md b/doc/integration/slack.md index f84ab769218..9fcf2c2d99a 100644 --- a/doc/integration/slack.md +++ b/doc/integration/slack.md @@ -1,5 +1,5 @@ --- -redirect_to: '../project_services/slack.md' +redirect_to: '../user/project/integrations/slack.md' --- -This document was moved to [project_services/slack.md](../project_services/slack.md). +This document was moved to [project_services/slack.md](../user/project/integrations/slack.md). -- cgit v1.2.1 From f5ec6b4e1296475aa4930b504ed2e3d329631fcb Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Tue, 23 Jul 2019 17:30:25 +0800 Subject: Remove project from show_label_issuables_link? The project param is unnecessary here --- app/helpers/labels_helper.rb | 5 ++--- app/views/shared/_label_row.html.haml | 4 ++-- spec/helpers/labels_helper_spec.rb | 24 +++--------------------- 3 files changed, 7 insertions(+), 26 deletions(-) diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index db4f29cd996..2ed016beea4 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -4,11 +4,10 @@ module LabelsHelper extend self include ActionView::Helpers::TagHelper - def show_label_issuables_link?(label, issuables_type, current_user: nil, project: nil) + def show_label_issuables_link?(label, issuables_type, current_user: nil) return true unless label.project_label? - return true unless project - project.feature_available?(issuables_type, current_user) + label.project.feature_available?(issuables_type, current_user) end # Link to a Label diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml index af11ce94ec5..b05d903fabe 100644 --- a/app/views/shared/_label_row.html.haml +++ b/app/views/shared/_label_row.html.haml @@ -1,7 +1,7 @@ - force_priority = local_assigns.fetch(:force_priority, false) - subject_or_group_defined = defined?(@project) || defined?(@group) -- show_label_issues_link = subject_or_group_defined && show_label_issuables_link?(label, :issues, project: @project) -- show_label_merge_requests_link = subject_or_group_defined && show_label_issuables_link?(label, :merge_requests, project: @project) +- show_label_issues_link = subject_or_group_defined && show_label_issuables_link?(label, :issues) +- show_label_merge_requests_link = subject_or_group_defined && show_label_issuables_link?(label, :merge_requests) .label-name = render_label(label, tooltip: false) diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 314305d7a8e..4f1cab38f34 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -3,10 +3,8 @@ require 'spec_helper' describe LabelsHelper do describe '#show_label_issuables_link?' do shared_examples 'a valid response to show_label_issuables_link?' do |issuables_type, when_enabled = true, when_disabled = false| - let(:context_project) { project } - context "when asking for a #{issuables_type} link" do - subject { show_label_issuables_link?(label.present(issuable_subject: nil), issuables_type, project: context_project) } + subject { show_label_issuables_link?(label.present(issuable_subject: nil), issuables_type) } context "when #{issuables_type} are enabled for the project" do let(:project) { create(:project, "#{issuables_type}_access_level": ProjectFeature::ENABLED) } @@ -39,27 +37,11 @@ describe LabelsHelper do let(:label) { create(:group_label, group: group, title: 'bug') } context 'when asking for an issue link' do - context 'in the context of a project' do - it_behaves_like 'a valid response to show_label_issuables_link?', :issues, true, true - end - - context 'in the context of a group' do - let(:context_project) { nil } - - it_behaves_like 'a valid response to show_label_issuables_link?', :issues, true, true - end + it_behaves_like 'a valid response to show_label_issuables_link?', :issues, true, true end context 'when asking for a merge requests link' do - context 'in the context of a project' do - it_behaves_like 'a valid response to show_label_issuables_link?', :merge_requests, true, true - end - - context 'in the context of a group' do - let(:context_project) { nil } - - it_behaves_like 'a valid response to show_label_issuables_link?', :merge_requests, true, true - end + it_behaves_like 'a valid response to show_label_issuables_link?', :merge_requests, true, true end end end -- cgit v1.2.1 From 1f403720293393f0f0bf6f99476abe4ba79580a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 15 Jul 2019 19:15:10 +0200 Subject: Do not authorize with OAuth for CICD only projects --- app/controllers/import/github_controller.rb | 8 ++++++-- spec/controllers/import/github_controller_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index aa4aa0fbdac..ebb50fc8b10 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -10,7 +10,7 @@ class Import::GithubController < Import::BaseController rescue_from Octokit::Unauthorized, with: :provider_unauthorized def new - if github_import_configured? && logged_in_with_provider? + if !ci_cd_only? && github_import_configured? && logged_in_with_provider? go_to_provider_for_permissions elsif session[access_token_key] redirect_to status_import_url @@ -169,11 +169,15 @@ class Import::GithubController < Import::BaseController # rubocop: enable CodeReuse/ActiveRecord def provider_auth - if session[access_token_key].blank? + if !ci_cd_only? && session[access_token_key].blank? go_to_provider_for_permissions end end + def ci_cd_only? + %w[1 true].include?(params[:ci_cd_only]) + end + def client_options {} end diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 059354870b5..5675798ac33 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -33,6 +33,16 @@ describe Import::GithubController do expect(response).to have_http_status(200) end + + context 'when importing a CI/CD project' do + it 'always prompts for an access token' do + allow(controller).to receive(:github_import_configured?).and_return(true) + + get :new, params: { ci_cd_only: true } + + expect(response).to render_template(:new) + end + end end describe "GET callback" do -- cgit v1.2.1 From 982d5582ee4d5b91f008093e7c7d30d8a103d518 Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Tue, 23 Jul 2019 18:06:51 +0300 Subject: Update author prefix on merge request notification --- app/views/notify/new_merge_request_email.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 2ddea0b9f16..8fecdc6e8a6 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -5,7 +5,7 @@ .branch = merge_path_description(@merge_request, 'to') .author - Author #{@merge_request.author_name} + Author: #{@merge_request.author_name} .assignee = assignees_label(@merge_request) .approvers -- cgit v1.2.1 From b4f695da6b798d2b027edaf21446e0cd68e1a55a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 23 Jul 2019 17:53:17 +0200 Subject: Increase the Review App deploy timeout to 15 minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- scripts/review_apps/review-apps.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index bc47884ee45..2b565347f86 100755 --- a/scripts/review_apps/review-apps.sh +++ b/scripts/review_apps/review-apps.sh @@ -201,7 +201,7 @@ function deploy() { HELM_CMD=$(cat << EOF helm upgrade --install \ --wait \ - --timeout 600 \ + --timeout 900 \ --set releaseOverride="$CI_ENVIRONMENT_SLUG" \ --set global.appConfig.enableUsagePing=false \ --set global.imagePullPolicy=Always \ -- cgit v1.2.1 From 928b9a105516a53d956666449e8b42974ced3267 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 23 Jul 2019 10:28:23 -0700 Subject: Add documentation on how to add dependencies to yarn Danger constantly warns about de-duplicating dependencies, so put these instructions in the documentation so they can be searched. --- doc/development/new_fe_guide/dependencies.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/development/new_fe_guide/dependencies.md b/doc/development/new_fe_guide/dependencies.md index 12a4f089d41..8a6930acd37 100644 --- a/doc/development/new_fe_guide/dependencies.md +++ b/doc/development/new_fe_guide/dependencies.md @@ -15,6 +15,18 @@ Exceptions are made for some tools that we require in the `gitlab:assets:compile` CI job such as `webpack-bundle-analyzer` to analyze our production assets post-compile. +To add or upgrade a dependency, run: + +```sh +yarn add +``` + +This may introduce duplicate dependencies. To de-duplicate `yarn.lock`, run: + +```sh +node_modules/.bin/yarn-deduplicate --list --strategy fewer yarn.lock && yarn install +``` + --- > TODO: Add Dependencies -- cgit v1.2.1 From 271a6d1b8f447bcb14d9c134fa9d96e1791158b5 Mon Sep 17 00:00:00 2001 From: Jeremy Jackson Date: Tue, 23 Jul 2019 14:39:11 -0600 Subject: Resolves confusion within spec rake tasks --- lib/quality/test_level.rb | 1 + lib/tasks/spec.rake | 63 ++----------------------------------- spec/lib/quality/test_level_spec.rb | 4 +-- 3 files changed, 6 insertions(+), 62 deletions(-) diff --git a/lib/quality/test_level.rb b/lib/quality/test_level.rb index 24d8eac200c..60d79b52680 100644 --- a/lib/quality/test_level.rb +++ b/lib/quality/test_level.rb @@ -14,6 +14,7 @@ module Quality finders frontend graphql + haml_lint helpers initializers javascripts diff --git a/lib/tasks/spec.rake b/lib/tasks/spec.rake index c881ad4cf12..bf18332a8eb 100644 --- a/lib/tasks/spec.rake +++ b/lib/tasks/spec.rake @@ -2,8 +2,6 @@ return if Rails.env.production? -Rake::Task["spec"].clear if Rake::Task.task_defined?('spec') - namespace :spec do desc 'GitLab | RSpec | Run unit tests' RSpec::Core::RakeTask.new(:unit, :rspec_opts) do |t, args| @@ -26,63 +24,8 @@ namespace :spec do t.rspec_opts = args[:rspec_opts] end - desc '[Deprecated] Use the "bin/rspec --tag api" instead' - task :api do - cmds = [ - %w(rake gitlab:setup), - %w(rspec spec --tag @api) - ] - run_commands(cmds) - end - - desc '[Deprecated] Use the "spec:system" task instead' - task :feature do - cmds = [ - %w(rake gitlab:setup), - %w(rspec spec --tag @feature) - ] - run_commands(cmds) - end - - desc '[Deprecated] Use "bin/rspec spec/models" instead' - task :models do - cmds = [ - %w(rake gitlab:setup), - %w(rspec spec --tag @models) - ] - run_commands(cmds) - end - - desc '[Deprecated] Use "bin/rspec spec/services" instead' - task :services do - cmds = [ - %w(rake gitlab:setup), - %w(rspec spec --tag @services) - ] - run_commands(cmds) - end - - desc '[Deprecated] Use "bin/rspec spec/lib" instead' - task :lib do - cmds = [ - %w(rake gitlab:setup), - %w(rspec spec --tag @lib) - ] - run_commands(cmds) - end -end - -desc "GitLab | Run specs" -task :spec do - cmds = [ - %w(rake gitlab:setup), - %w(rspec spec) - ] - run_commands(cmds) -end - -def run_commands(cmds) - cmds.each do |cmd| - system({ 'RAILS_ENV' => 'test', 'force' => 'yes' }, *cmd) || raise("#{cmd} failed!") + desc 'Run the code examples in spec/requests/api' + RSpec::Core::RakeTask.new(:api) do |t| + t.pattern = 'spec/requests/api/**/*_spec.rb' end end diff --git a/spec/lib/quality/test_level_spec.rb b/spec/lib/quality/test_level_spec.rb index 3465c3a050b..59870ce44a7 100644 --- a/spec/lib/quality/test_level_spec.rb +++ b/spec/lib/quality/test_level_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Quality::TestLevel do context 'when level is unit' do it 'returns a pattern' do expect(subject.pattern(:unit)) - .to eq("spec/{bin,config,db,dependencies,factories,finders,frontend,graphql,helpers,initializers,javascripts,lib,migrations,models,policies,presenters,rack_servers,routing,rubocop,serializers,services,sidekiq,tasks,uploaders,validators,views,workers,elastic_integration}{,/**/}*_spec.rb") + .to eq("spec/{bin,config,db,dependencies,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,migrations,models,policies,presenters,rack_servers,routing,rubocop,serializers,services,sidekiq,tasks,uploaders,validators,views,workers,elastic_integration}{,/**/}*_spec.rb") end end @@ -47,7 +47,7 @@ RSpec.describe Quality::TestLevel do context 'when level is unit' do it 'returns a regexp' do expect(subject.regexp(:unit)) - .to eq(%r{spec/(bin|config|db|dependencies|factories|finders|frontend|graphql|helpers|initializers|javascripts|lib|migrations|models|policies|presenters|rack_servers|routing|rubocop|serializers|services|sidekiq|tasks|uploaders|validators|views|workers|elastic_integration)}) + .to eq(%r{spec/(bin|config|db|dependencies|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|migrations|models|policies|presenters|rack_servers|routing|rubocop|serializers|services|sidekiq|tasks|uploaders|validators|views|workers|elastic_integration)}) end end -- cgit v1.2.1 From 3e340b6ffc7f6c76fbbbabf580a96872b9eefa66 Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Thu, 18 Jul 2019 22:48:44 +1000 Subject: Update docs to reflect new Security tabs This commit includes changes to update the documentation so it reflects the new navigation structure introduced by the additional tab "Security & Compliance" to the project-views sidebar and "Security" to the group-views sidebar. * Screenshot * Paths to the Dependency List * Paths to the group-level security dashboard --- .../dependency_scanning/index.md | 2 +- .../security_dashboard/img/dashboard.png | Bin 58585 -> 0 bytes .../img/group_security_dashboard.png | Bin 0 -> 226261 bytes .../img/project_security_dashboard.png | Bin 126356 -> 166559 bytes .../security_dashboard/index.md | 4 ++-- 5 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 doc/user/application_security/security_dashboard/img/dashboard.png create mode 100644 doc/user/application_security/security_dashboard/img/group_security_dashboard.png diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index 7473647f129..a2f0584e8dc 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -331,7 +331,7 @@ project's dependencies with their versions. This list can be generated only for [languages and package managers](#supported-languages-and-package-managers) supported by Gemnasium. -To see the generated dependency list, navigate to your project's **Project > Dependency List**. +To see the generated dependency list, navigate to your project's **Security & Compliance > Dependency List**. ## Versioning and release process diff --git a/doc/user/application_security/security_dashboard/img/dashboard.png b/doc/user/application_security/security_dashboard/img/dashboard.png deleted file mode 100644 index a75168b1ce4..00000000000 Binary files a/doc/user/application_security/security_dashboard/img/dashboard.png and /dev/null differ diff --git a/doc/user/application_security/security_dashboard/img/group_security_dashboard.png b/doc/user/application_security/security_dashboard/img/group_security_dashboard.png new file mode 100644 index 00000000000..40689861e2a Binary files /dev/null and b/doc/user/application_security/security_dashboard/img/group_security_dashboard.png differ diff --git a/doc/user/application_security/security_dashboard/img/project_security_dashboard.png b/doc/user/application_security/security_dashboard/img/project_security_dashboard.png index f0dad6c54d0..89b310895d3 100644 Binary files a/doc/user/application_security/security_dashboard/img/project_security_dashboard.png and b/doc/user/application_security/security_dashboard/img/project_security_dashboard.png differ diff --git a/doc/user/application_security/security_dashboard/index.md b/doc/user/application_security/security_dashboard/index.md index 2a2385c00ae..4cd3fc5f735 100644 --- a/doc/user/application_security/security_dashboard/index.md +++ b/doc/user/application_security/security_dashboard/index.md @@ -49,7 +49,7 @@ The group Security Dashboard gives an overview of the vulnerabilities of all the projects in a group and its subgroups. First, navigate to the Security Dashboard found under your group's -**Overview > Security Dashboard**. +**Security** tab. Once you're on the dashboard, at the top you should see a series of filters for: @@ -58,7 +58,7 @@ Once you're on the dashboard, at the top you should see a series of filters for: - Report type - Project -![dashboard with action buttons and metrics](img/dashboard.png) +![dashboard with action buttons and metrics](img/group_security_dashboard.png) Selecting one or more filters will filter the results in this page. The first section is an overview of all the vulnerabilities, grouped by severity. -- cgit v1.2.1 From 13e7feef08a4b4fd32d553ac58f943a1f8a3d579 Mon Sep 17 00:00:00 2001 From: Patrick Derichs Date: Wed, 24 Jul 2019 08:53:29 +0200 Subject: Add where condition to filter out invalid labels with nil type --- app/models/label.rb | 2 +- changelogs/unreleased/63730-fix-500-status-labels-pd.yml | 5 +++++ spec/models/label_spec.rb | 13 +++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/63730-fix-500-status-labels-pd.yml diff --git a/app/models/label.rb b/app/models/label.rb index b83e0862bab..dd403562bfa 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -33,7 +33,7 @@ class Label < ApplicationRecord default_scope { order(title: :asc) } - scope :templates, -> { where(template: true) } + scope :templates, -> { where(template: true, type: [Label.name, nil]) } scope :with_title, ->(title) { where(title: title) } scope :with_lists_and_board, -> { joins(lists: :board).merge(List.movable) } scope :on_project_boards, ->(project_id) { with_lists_and_board.where(boards: { project_id: project_id }) } diff --git a/changelogs/unreleased/63730-fix-500-status-labels-pd.yml b/changelogs/unreleased/63730-fix-500-status-labels-pd.yml new file mode 100644 index 00000000000..a1e2ae0e5df --- /dev/null +++ b/changelogs/unreleased/63730-fix-500-status-labels-pd.yml @@ -0,0 +1,5 @@ +--- +title: Fix admin labels page when there are invalid records +merge_request: 30885 +author: +type: fixed diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 5174c590a10..c2e2298823e 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -193,4 +193,17 @@ describe Label do expect(described_class.optionally_subscribed_by(nil)).to match_array([label, label2]) end end + + describe '#templates' do + context 'with invalid template labels' do + it 'returns only valid template labels' do + create(:label) + # Project labels should not have template set to true + create(:label, template: true) + valid_template_label = described_class.create!(title: 'test', template: true, type: nil) + + expect(described_class.templates).to eq([valid_template_label]) + end + end + end end -- cgit v1.2.1 From 74f0faa27c12a50f7705161b66c27485809e8e07 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 24 Jul 2019 12:49:43 +0100 Subject: Remove unused peek view code We haven't needed these since we started rendering the performance bar in Vue. --- app/views/peek/views/_gc.html.haml | 7 ------- app/views/peek/views/_redis.html.haml | 7 ------- app/views/peek/views/_sidekiq.html.haml | 7 ------- locale/gitlab.pot | 6 ------ 4 files changed, 27 deletions(-) delete mode 100644 app/views/peek/views/_gc.html.haml delete mode 100644 app/views/peek/views/_redis.html.haml delete mode 100644 app/views/peek/views/_sidekiq.html.haml diff --git a/app/views/peek/views/_gc.html.haml b/app/views/peek/views/_gc.html.haml deleted file mode 100644 index 2a586261ce1..00000000000 --- a/app/views/peek/views/_gc.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -- local_assigns.fetch(:view) - -%span.bold - %span{ title: _('Invoke Time'), data: { defer_to: "#{view.defer_key}-gc_time" } }... - \/ - %span{ title: _('Invoke Count'), data: { defer_to: "#{view.defer_key}-invokes" } }... -gc diff --git a/app/views/peek/views/_redis.html.haml b/app/views/peek/views/_redis.html.haml deleted file mode 100644 index f7fba6c95fc..00000000000 --- a/app/views/peek/views/_redis.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -- local_assigns.fetch(:view) - -%span.bold - %span{ data: { defer_to: "#{view.defer_key}-duration" } }... - \/ - %span{ data: { defer_to: "#{view.defer_key}-calls" } }... -redis diff --git a/app/views/peek/views/_sidekiq.html.haml b/app/views/peek/views/_sidekiq.html.haml deleted file mode 100644 index 7efbc05890d..00000000000 --- a/app/views/peek/views/_sidekiq.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -- local_assigns.fetch(:view) - -%span.bold - %span{ data: { defer_to: "#{view.defer_key}-duration" } }... - \/ - %span{ data: { defer_to: "#{view.defer_key}-calls" } }... -sidekiq diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7b1a3fd4d2c..97defd42742 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5696,12 +5696,6 @@ msgstr "" msgid "Invocations" msgstr "" -msgid "Invoke Count" -msgstr "" - -msgid "Invoke Time" -msgstr "" - msgid "IssuableStatus|Closed (%{moved_link_start}moved%{moved_link_end})" msgstr "" -- cgit v1.2.1 From f36da45711f7d412951b50c6b9db913a5e324171 Mon Sep 17 00:00:00 2001 From: Adam Hegyi Date: Mon, 22 Jul 2019 09:47:29 +0200 Subject: Make RelativePositioning reusable RelativePositioning module was heavily dependent on the Issue model. This changes makes it easier to reuse the functionality provided by RelativePositioning in other models. Needed by: https://gitlab.com/gitlab-org/gitlab-ee/issues/12196 --- app/models/concerns/relative_positioning.rb | 52 +++-- app/models/issue.rb | 6 +- spec/models/concerns/relative_positioning_spec.rb | 242 -------------------- spec/models/issue_spec.rb | 8 + .../relative_positioning_shared_examples.rb | 253 +++++++++++++++++++++ 5 files changed, 300 insertions(+), 261 deletions(-) delete mode 100644 spec/models/concerns/relative_positioning_spec.rb create mode 100644 spec/support/shared_examples/relative_positioning_shared_examples.rb diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index e4fe46d722a..9cd7b8d6258 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -1,5 +1,26 @@ # frozen_string_literal: true +# This module makes it possible to handle items as a list, where the order of items can be easily altered +# Requirements: +# +# - Only works for ActiveRecord models +# - relative_position integer field must present on the model +# - This module uses GROUP BY: the model should have a parent relation, example: project -> issues, project is the parent relation (issues table has a parent_id column) +# +# Setup like this in the body of your class: +# +# include RelativePositioning +# +# # base query used for the position calculation +# def self.relative_positioning_query_base(issue) +# where(deleted: false) +# end +# +# # column that should be used in GROUP BY +# def self.relative_positioning_parent_column +# :project_id +# end +# module RelativePositioning extend ActiveSupport::Concern @@ -93,7 +114,7 @@ module RelativePositioning return move_after(before) unless after return move_before(after) unless before - # If there is no place to insert an issue we need to create one by moving the before issue closer + # If there is no place to insert an item we need to create one by moving the before item closer # to its predecessor. This process will recursively move all the predecessors until we have a place if (after.relative_position - before.relative_position) < 2 before.move_before @@ -108,11 +129,11 @@ module RelativePositioning pos_after = before.next_relative_position if before.shift_after? - issue_to_move = self.class.in_parents(parent_ids).find_by!(relative_position: pos_after) - issue_to_move.move_after - @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables + item_to_move = self.class.relative_positioning_query_base(self).find_by!(relative_position: pos_after) + item_to_move.move_after + @positionable_neighbours = [item_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables - pos_after = issue_to_move.relative_position + pos_after = item_to_move.relative_position end self.relative_position = self.class.position_between(pos_before, pos_after) @@ -123,11 +144,11 @@ module RelativePositioning pos_before = after.prev_relative_position if after.shift_before? - issue_to_move = self.class.in_parents(parent_ids).find_by!(relative_position: pos_before) - issue_to_move.move_before - @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables + item_to_move = self.class.relative_positioning_query_base(self).find_by!(relative_position: pos_before) + item_to_move.move_before + @positionable_neighbours = [item_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables - pos_before = issue_to_move.relative_position + pos_before = item_to_move.relative_position end self.relative_position = self.class.position_between(pos_before, pos_after) @@ -141,13 +162,13 @@ module RelativePositioning self.relative_position = self.class.position_between(min_relative_position || START_POSITION, MIN_POSITION) end - # Indicates if there is an issue that should be shifted to free the place + # Indicates if there is an item that should be shifted to free the place def shift_after? next_pos = next_relative_position next_pos && (next_pos - relative_position) == 1 end - # Indicates if there is an issue that should be shifted to free the place + # Indicates if there is an item that should be shifted to free the place def shift_before? prev_pos = prev_relative_position prev_pos && (relative_position - prev_pos) == 1 @@ -159,7 +180,7 @@ module RelativePositioning def save_positionable_neighbours return unless @positionable_neighbours - status = @positionable_neighbours.all? { |issue| issue.save(touch: false) } + status = @positionable_neighbours.all? { |item| item.save(touch: false) } @positionable_neighbours = nil status @@ -170,16 +191,15 @@ module RelativePositioning # When calculating across projects, this is much more efficient than # MAX(relative_position) without the GROUP BY, due to index usage: # https://gitlab.com/gitlab-org/gitlab-ce/issues/54276#note_119340977 - relation = self.class - .in_parents(parent_ids) + relation = self.class.relative_positioning_query_base(self) .order(Gitlab::Database.nulls_last_order('position', 'DESC')) + .group(self.class.relative_positioning_parent_column) .limit(1) - .group(self.class.parent_column) relation = yield relation if block_given? relation - .pluck(self.class.parent_column, Arel.sql("#{calculation}(relative_position) AS position")) + .pluck(self.class.relative_positioning_parent_column, Arel.sql("#{calculation}(relative_position) AS position")) .first&. last end diff --git a/app/models/issue.rb b/app/models/issue.rb index 8c5dd5e382e..164858dc432 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -91,11 +91,11 @@ class Issue < ApplicationRecord end end - class << self - alias_method :in_parents, :in_projects + def self.relative_positioning_query_base(issue) + in_projects(issue.parent_ids) end - def self.parent_column + def self.relative_positioning_parent_column :project_id end diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb deleted file mode 100644 index d0ae45f7871..00000000000 --- a/spec/models/concerns/relative_positioning_spec.rb +++ /dev/null @@ -1,242 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe RelativePositioning do - let(:project) { create(:project) } - let(:issue) { create(:issue, project: project) } - let(:issue1) { create(:issue, project: project) } - let(:new_issue) { create(:issue, project: project) } - - describe '.move_to_end' do - it 'moves the object to the end' do - Issue.move_to_end([issue, issue1]) - - expect(issue1.prev_relative_position).to eq issue.relative_position - expect(issue.prev_relative_position).to eq nil - expect(issue1.next_relative_position).to eq nil - end - - it 'does not perform any moves if all issues have their relative_position set' do - issue.update!(relative_position: 1) - - expect(issue).not_to receive(:save) - - Issue.move_to_end([issue]) - end - end - - describe '#max_relative_position' do - it 'returns maximum position' do - expect(issue.max_relative_position).to eq issue1.relative_position - end - end - - describe '#prev_relative_position' do - it 'returns previous position if there is an issue above' do - expect(issue1.prev_relative_position).to eq issue.relative_position - end - - it 'returns nil if there is no issue above' do - expect(issue.prev_relative_position).to eq nil - end - end - - describe '#next_relative_position' do - it 'returns next position if there is an issue below' do - expect(issue.next_relative_position).to eq issue1.relative_position - end - - it 'returns nil if there is no issue below' do - expect(issue1.next_relative_position).to eq nil - end - end - - describe '#move_before' do - it 'moves issue before' do - [issue1, issue].each(&:move_to_end) - - issue.move_before(issue1) - - expect(issue.relative_position).to be < issue1.relative_position - end - end - - describe '#move_after' do - it 'moves issue after' do - [issue, issue1].each(&:move_to_end) - - issue.move_after(issue1) - - expect(issue.relative_position).to be > issue1.relative_position - end - end - - describe '#move_to_end' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'moves issue to the end' do - new_issue.move_to_end - - expect(new_issue.relative_position).to be > issue1.relative_position - end - end - - describe '#shift_after?' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'returns true' do - issue.update(relative_position: issue1.relative_position - 1) - - expect(issue.shift_after?).to be_truthy - end - - it 'returns false' do - issue.update(relative_position: issue1.relative_position - 2) - - expect(issue.shift_after?).to be_falsey - end - end - - describe '#shift_before?' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'returns true' do - issue.update(relative_position: issue1.relative_position + 1) - - expect(issue.shift_before?).to be_truthy - end - - it 'returns false' do - issue.update(relative_position: issue1.relative_position + 2) - - expect(issue.shift_before?).to be_falsey - end - end - - describe '#move_between' do - before do - [issue, issue1].each do |issue| - issue.move_to_end && issue.save - end - end - - it 'positions issue between two other' do - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be > issue.relative_position - expect(new_issue.relative_position).to be < issue1.relative_position - end - - it 'positions issue between on top' do - new_issue.move_between(nil, issue) - - expect(new_issue.relative_position).to be < issue.relative_position - end - - it 'positions issue between to end' do - new_issue.move_between(issue1, nil) - - expect(new_issue.relative_position).to be > issue1.relative_position - end - - it 'positions issues even when after and before positions are the same' do - issue1.update relative_position: issue.relative_position - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be > issue.relative_position - expect(issue.relative_position).to be < issue1.relative_position - end - - it 'positions issues between other two if distance is 1' do - issue1.update relative_position: issue.relative_position + 1 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be > issue.relative_position - expect(issue.relative_position).to be < issue1.relative_position - end - - it 'positions issue in the middle of other two if distance is big enough' do - issue.update relative_position: 6000 - issue1.update relative_position: 10000 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to eq(8000) - end - - it 'positions issue closer to the middle if we are at the very top' do - issue1.update relative_position: 6000 - - new_issue.move_between(nil, issue1) - - expect(new_issue.relative_position).to eq(6000 - RelativePositioning::IDEAL_DISTANCE) - end - - it 'positions issue closer to the middle if we are at the very bottom' do - issue.update relative_position: 6000 - issue1.update relative_position: nil - - new_issue.move_between(issue, nil) - - expect(new_issue.relative_position).to eq(6000 + RelativePositioning::IDEAL_DISTANCE) - end - - it 'positions issue in the middle of other two if distance is not big enough' do - issue.update relative_position: 100 - issue1.update relative_position: 400 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to eq(250) - end - - it 'positions issue in the middle of other two is there is no place' do - issue.update relative_position: 100 - issue1.update relative_position: 101 - - new_issue.move_between(issue, issue1) - - expect(new_issue.relative_position).to be_between(issue.relative_position, issue1.relative_position) - end - - it 'uses rebalancing if there is no place' do - issue.update relative_position: 100 - issue1.update relative_position: 101 - issue2 = create(:issue, relative_position: 102, project: project) - new_issue.update relative_position: 103 - - new_issue.move_between(issue1, issue2) - new_issue.save! - - expect(new_issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) - expect(issue.reload.relative_position).not_to eq(100) - end - - it 'positions issue right if we pass none-sequential parameters' do - issue.update relative_position: 99 - issue1.update relative_position: 101 - issue2 = create(:issue, relative_position: 102, project: project) - new_issue.update relative_position: 103 - - new_issue.move_between(issue, issue2) - new_issue.save! - - expect(new_issue.relative_position).to be(100) - end - end -end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index d5b016dc8f6..2e7d78d77a8 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -871,4 +871,12 @@ describe Issue do expect(issue.labels_hook_attrs).to eq([label.hook_attrs]) end end + + context "relative positioning" do + it_behaves_like "a class that supports relative positioning" do + let(:project) { create(:project) } + let(:factory) { :issue } + let(:default_params) { { project: project } } + end + end end diff --git a/spec/support/shared_examples/relative_positioning_shared_examples.rb b/spec/support/shared_examples/relative_positioning_shared_examples.rb new file mode 100644 index 00000000000..5ee62644c54 --- /dev/null +++ b/spec/support/shared_examples/relative_positioning_shared_examples.rb @@ -0,0 +1,253 @@ +# frozen_string_literal: true + +RSpec.shared_examples "a class that supports relative positioning" do + let(:item1) { create(factory, default_params) } + let(:item2) { create(factory, default_params) } + let(:new_item) { create(factory, default_params) } + + def create_item(params) + create(factory, params.merge(default_params)) + end + + describe '.move_to_end' do + it 'moves the object to the end' do + item1.update(relative_position: 5) + item2.update(relative_position: 15) + + described_class.move_to_end([item1, item2]) + + expect(item2.prev_relative_position).to eq item1.relative_position + expect(item1.prev_relative_position).to eq nil + expect(item2.next_relative_position).to eq nil + end + + it 'does not perform any moves if all items have their relative_position set' do + item1.update!(relative_position: 1) + + expect(item1).not_to receive(:save) + + described_class.move_to_end([item1]) + end + end + + describe '#max_relative_position' do + it 'returns maximum position' do + expect(item1.max_relative_position).to eq item2.relative_position + end + end + + describe '#prev_relative_position' do + it 'returns previous position if there is an item above' do + item1.update(relative_position: 5) + item2.update(relative_position: 15) + + expect(item2.prev_relative_position).to eq item1.relative_position + end + + it 'returns nil if there is no item above' do + expect(item1.prev_relative_position).to eq nil + end + end + + describe '#next_relative_position' do + it 'returns next position if there is an item below' do + item1.update(relative_position: 5) + item2.update(relative_position: 15) + + expect(item1.next_relative_position).to eq item2.relative_position + end + + it 'returns nil if there is no item below' do + expect(item2.next_relative_position).to eq nil + end + end + + describe '#move_before' do + it 'moves item before' do + [item2, item1].each(&:move_to_end) + + item1.move_before(item2) + + expect(item1.relative_position).to be < item2.relative_position + end + end + + describe '#move_after' do + it 'moves item after' do + [item1, item2].each(&:move_to_end) + + item1.move_after(item2) + + expect(item1.relative_position).to be > item2.relative_position + end + end + + describe '#move_to_end' do + before do + [item1, item2].each do |item1| + item1.move_to_end && item1.save + end + end + + it 'moves item to the end' do + new_item.move_to_end + + expect(new_item.relative_position).to be > item2.relative_position + end + end + + describe '#shift_after?' do + before do + [item1, item2].each do |item1| + item1.move_to_end && item1.save + end + end + + it 'returns true' do + item1.update(relative_position: item2.relative_position - 1) + + expect(item1.shift_after?).to be_truthy + end + + it 'returns false' do + item1.update(relative_position: item2.relative_position - 2) + + expect(item1.shift_after?).to be_falsey + end + end + + describe '#shift_before?' do + before do + [item1, item2].each do |item1| + item1.move_to_end && item1.save + end + end + + it 'returns true' do + item1.update(relative_position: item2.relative_position + 1) + + expect(item1.shift_before?).to be_truthy + end + + it 'returns false' do + item1.update(relative_position: item2.relative_position + 2) + + expect(item1.shift_before?).to be_falsey + end + end + + describe '#move_between' do + before do + [item1, item2].each do |item1| + item1.move_to_end && item1.save + end + end + + it 'positions item between two other' do + new_item.move_between(item1, item2) + + expect(new_item.relative_position).to be > item1.relative_position + expect(new_item.relative_position).to be < item2.relative_position + end + + it 'positions item between on top' do + new_item.move_between(nil, item1) + + expect(new_item.relative_position).to be < item1.relative_position + end + + it 'positions item between to end' do + new_item.move_between(item2, nil) + + expect(new_item.relative_position).to be > item2.relative_position + end + + it 'positions items even when after and before positions are the same' do + item2.update relative_position: item1.relative_position + + new_item.move_between(item1, item2) + + expect(new_item.relative_position).to be > item1.relative_position + expect(item1.relative_position).to be < item2.relative_position + end + + it 'positions items between other two if distance is 1' do + item2.update relative_position: item1.relative_position + 1 + + new_item.move_between(item1, item2) + + expect(new_item.relative_position).to be > item1.relative_position + expect(item1.relative_position).to be < item2.relative_position + end + + it 'positions item in the middle of other two if distance is big enough' do + item1.update relative_position: 6000 + item2.update relative_position: 10000 + + new_item.move_between(item1, item2) + + expect(new_item.relative_position).to eq(8000) + end + + it 'positions item closer to the middle if we are at the very top' do + item2.update relative_position: 6000 + + new_item.move_between(nil, item2) + + expect(new_item.relative_position).to eq(6000 - RelativePositioning::IDEAL_DISTANCE) + end + + it 'positions item closer to the middle if we are at the very bottom' do + new_item.update relative_position: 1 + item1.update relative_position: 6000 + item2.destroy + + new_item.move_between(item1, nil) + + expect(new_item.relative_position).to eq(6000 + RelativePositioning::IDEAL_DISTANCE) + end + + it 'positions item in the middle of other two if distance is not big enough' do + item1.update relative_position: 100 + item2.update relative_position: 400 + + new_item.move_between(item1, item2) + + expect(new_item.relative_position).to eq(250) + end + + it 'positions item in the middle of other two is there is no place' do + item1.update relative_position: 100 + item2.update relative_position: 101 + + new_item.move_between(item1, item2) + + expect(new_item.relative_position).to be_between(item1.relative_position, item2.relative_position) + end + + it 'uses rebalancing if there is no place' do + item1.update relative_position: 100 + item2.update relative_position: 101 + item3 = create_item(relative_position: 102) + new_item.update relative_position: 103 + + new_item.move_between(item2, item3) + new_item.save! + + expect(new_item.relative_position).to be_between(item2.relative_position, item3.relative_position) + expect(item1.reload.relative_position).not_to eq(100) + end + + it 'positions item right if we pass none-sequential parameters' do + item1.update relative_position: 99 + item2.update relative_position: 101 + item3 = create_item(relative_position: 102) + new_item.update relative_position: 103 + + new_item.move_between(item1, item3) + new_item.save! + + expect(new_item.relative_position).to be(100) + end + end +end -- cgit v1.2.1 From 105bb6ffca3d53b81ee8d1cce6521ed28c2b1ba7 Mon Sep 17 00:00:00 2001 From: Steve Azzopardi Date: Wed, 24 Jul 2019 12:58:15 +0000 Subject: Add Go test guidelines --- doc/development/go_guide/index.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index f827d240bf6..9f0ac8cc753 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -173,6 +173,45 @@ func Test(t *testing.T) { } ``` +### Table-Driven Tests + +Using [Table-Driven Tests](https://github.com/golang/go/wiki/TableDrivenTests) +is generally good practice when you have multiple entries of +inputs/outputs for the same function. Below are some guidelines one can +follow when writing table-driven test. These guidelines are mostly +extracted from Go standard library source code. Keep in mind it's OK not +to follow these guidelines when it makes sense. + +#### Defining test cases + +Each table entry is a complete test case with inputs and expected +results, and sometimes with additional information such as a test name +to make the test output easily readable. + +- [Define a slice of anonymous struct](https://github.com/golang/go/blob/50bd1c4d4eb4fac8ddeb5f063c099daccfb71b26/src/encoding/csv/reader_test.go#L16) + inside of the test. +- [Define a slice of anonymous struct](https://github.com/golang/go/blob/55d31e16c12c38d36811bdee65ac1f7772148250/src/cmd/go/internal/module/module_test.go#L9-L66) + outside of the test. +- [Named structs](https://github.com/golang/go/blob/2e0cd2aef5924e48e1ceb74e3d52e76c56dd34cc/src/cmd/go/internal/modfetch/coderepo_test.go#L54-L69) + for code reuse. +- [Using `map[string]struct{}`](https://github.com/golang/go/blob/6d5caf38e37bf9aeba3291f1f0b0081f934b1187/src/cmd/trace/annotations_test.go#L180-L235). + +#### Contents of the test case + +- Ideally, each test case should have a field with a unique identifier + to use for naming subtests. In the Go standard library, this is commonly the + `name string` field. +- Use `want`/`expect`/`actual` when you are specifcing something in the + test case that will be used for assertion. + +#### Variable names + +- Each table-driven test map/slice of struct can be named `tests`. +- When looping through `tests` the anonymous struct can be referred + to as `tt` or `tc`. +- The description of the test can be referred to as + `name`/`testName`/`tn`. + ### Benchmarks Programs handling a lot of IO or complex operations should always include -- cgit v1.2.1 From ea2a64c3ace77b23420e6bb60492af03492e0aa4 Mon Sep 17 00:00:00 2001 From: Evan Read Date: Wed, 24 Jul 2019 13:07:42 +0000 Subject: Improve code quality documentation --- doc/user/project/merge_requests/code_quality.md | 79 ++++++++++++++++++------- 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/doc/user/project/merge_requests/code_quality.md b/doc/user/project/merge_requests/code_quality.md index ad1d79ae5b1..eb6e454062a 100644 --- a/doc/user/project/merge_requests/code_quality.md +++ b/doc/user/project/merge_requests/code_quality.md @@ -9,8 +9,18 @@ in [GitLab Starter](https://about.gitlab.com/pricing/) 9.3. With the help of [GitLab CI/CD](../../../ci/README.md), you can analyze your source code quality using GitLab Code Quality. -Code Quality uses [Code Climate Engines](https://codeclimate.com), which are -free and open source. Code Quality doesn't require a Code Climate subscription. + +Code Quality: + +- Uses [Code Climate Engines](https://codeclimate.com), which are + free and open source. Code Quality doesn't require a Code Climate + subscription. +- Runs in [pipelines](../../../ci/pipelines.md) using an Docker image built in + [GitLab Code + Quality](https://gitlab.com/gitlab-org/security-products/codequality) project. +- Can make use of a [template](#template-and-examples). +- Is available with [Auto + DevOps](../../../topics/autodevops/index.md#auto-code-quality-starter). Going a step further, GitLab can show the Code Quality report right in the merge request widget area: @@ -21,22 +31,48 @@ in the merge request widget area: For instance, consider the following workflow: -1. Your backend team member starts a new implementation for making a certain feature in your app faster -1. With Code Quality reports, they analyze how their implementation is impacting the code quality -1. The metrics show that their code degrade the quality in 10 points -1. You ask a co-worker to help them with this modification -1. They both work on the changes until Code Quality report displays no degradations, only improvements -1. You approve the merge request and authorize its deployment to staging -1. Once verified, their changes are deployed to production +1. Your backend team member starts a new implementation for making a certain + feature in your app faster. +1. With Code Quality reports, they analyze how their implementation is impacting + the code quality. +1. The metrics show that their code degrade the quality in 10 points. +1. You ask a co-worker to help them with this modification. +1. They both work on the changes until Code Quality report displays no + degradations, only improvements. +1. You approve the merge request and authorize its deployment to staging. +1. Once verified, their changes are deployed to production. + +## Template and examples + +For most GitLab instances, the supplied template is the preferred method of +implementing Code Quality. See +[Analyze your project's Code Quality](../../../ci/examples/code_quality.md) for: + +- Information on the builtin GitLab Code Quality template. +- Examples of manual GitLab configuration for earlier GitLab versions. -## How it works +## Configuring jobs using variables -First of all, you need to define a job in your `.gitlab-ci.yml` file that generates the -[Code Quality report artifact](../../../ci/yaml/README.md#artifactsreportscodequality-starter). +The Code Quality job supports environment variables that users can set to +configure job execution at runtime. -The Code Quality report artifact is a subset of the -[Code Climate spec](https://github.com/codeclimate/spec/blob/master/SPEC.md#data-types). -It must be a JSON file containing an array of objects with the following properties: +For a list of available environment variables, see +[Environment variables](https://gitlab.com/gitlab-org/security-products/codequality/blob/master/README.md#environment-variables). + +## Implementing a custom tool + +It's possible to have a custom tool provide Code Quality reports in GitLab. To +do this: + +1. Define a job in your `.gitlab-ci.yml` file that generates the + [Code Quality report + artifact](../../../ci/yaml/README.md#artifactsreportscodequality-starter). +1. Configure your tool to generate the Code Quality report artifact as a JSON + file that implements subset of the [Code Climate + spec](https://github.com/codeclimate/spec/blob/master/SPEC.md#data-types). + +The Code Quality report artifact JSON file must contain an array of objects +with the following properties: | Name | Description | | ---------------------- | -------------------------------------------------------------------------------------- | @@ -63,13 +99,16 @@ Example: ``` NOTE: **Note:** -Although the Code Climate spec supports more properties, those are ignored by GitLab. +Although the Code Climate spec supports more properties, those are ignored by +GitLab. + +## Code Quality reports -For more information on what the Code Quality job should look like, check the -example on [analyzing a project's code quality](../../../ci/examples/code_quality.md). +Once the Code Quality job has completed, GitLab: -GitLab then checks this report, compares the metrics between the source and target -branches, and shows the information right on the merge request. +- Checks the generated report. +- Compares the metrics between the source and target branches. +- Shows the information right on the merge request. If multiple jobs in a pipeline generate a code quality artifact, only the artifact from the last created job (the job with the largest job ID) is used. To avoid confusion, -- cgit v1.2.1 From 74a34e8b7bd3019b63eeae58abc7185c122bc528 Mon Sep 17 00:00:00 2001 From: Marcel Amirault Date: Wed, 24 Jul 2019 13:10:06 +0000 Subject: Clean up headers in markdown Some markdown headers needed tweaking to adhere to standards, including blank lines above and below, only one space after hash, first header should be h1, and only one h1 per doc --- doc/administration/troubleshooting/debug.md | 2 +- doc/api/epics.md | 2 +- doc/ci/pipelines.md | 2 +- doc/development/contributing/community_roles.md | 2 +- doc/development/fe_guide/axios.md | 3 +++ doc/development/fe_guide/style_guide_scss.md | 1 + doc/development/module_with_instance_variables.md | 20 ++++++++++---------- .../new_fe_guide/development/accessibility.md | 5 +++++ doc/development/new_fe_guide/style/index.md | 2 +- doc/development/verifying_database_capabilities.md | 2 +- doc/user/admin_area/settings/usage_statistics.md | 4 ++-- .../integrations/prometheus_library/haproxy.md | 1 + 12 files changed, 28 insertions(+), 18 deletions(-) diff --git a/doc/administration/troubleshooting/debug.md b/doc/administration/troubleshooting/debug.md index 098d946a9fa..604dff5983d 100644 --- a/doc/administration/troubleshooting/debug.md +++ b/doc/administration/troubleshooting/debug.md @@ -209,7 +209,7 @@ ps auwx | grep unicorn | awk '{ print " -p " $2}' | xargs strace -tt -T -f -s 10 The output in `/tmp/unicorn.txt` may help diagnose the root cause. -# More information +## More information - [Debugging Stuck Ruby Processes](https://blog.newrelic.com/2013/04/29/debugging-stuck-ruby-processes-what-to-do-before-you-kill-9/) - [Cheatsheet of using gdb and ruby processes](gdb-stuck-ruby.txt) diff --git a/doc/api/epics.md b/doc/api/epics.md index d05eb0a8804..3036b3c2364 100644 --- a/doc/api/epics.md +++ b/doc/api/epics.md @@ -10,7 +10,7 @@ If epics feature is not available a `403` status code will be returned. The [epic issues API](epic_issues.md) allows you to interact with issues associated with an epic. -# Milestone dates integration +## Milestone dates integration > [Introduced][ee-6448] in GitLab 11.3. diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index 06a81c3d0c7..98f30350968 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -345,7 +345,7 @@ GitLab provides API endpoints to: - [Triggering pipelines through the API](triggers/README.md). - [Pipeline triggers API](../api/pipeline_triggers.md). -### Start multiple manual actions in a stage +### Start multiple manual actions in a stage > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27188) in GitLab 11.11. diff --git a/doc/development/contributing/community_roles.md b/doc/development/contributing/community_roles.md index 3296cb173d7..7d2d1b77a0e 100644 --- a/doc/development/contributing/community_roles.md +++ b/doc/development/contributing/community_roles.md @@ -1,4 +1,4 @@ -### Community members & roles +# Community members & roles GitLab community members and their privileges/responsibilities. diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md index 0d9397c3bd5..09b4a3c3d96 100644 --- a/doc/development/fe_guide/axios.md +++ b/doc/development/fe_guide/axios.md @@ -1,15 +1,18 @@ # Axios + We use [axios][axios] to communicate with the server in Vue applications and most new code. In order to guarantee all defaults are set you *should not use `axios` directly*, you should import `axios` from `axios_utils`. ## CSRF token + All our request require a CSRF token. To guarantee this token is set, we are importing [axios][axios], setting the token, and exporting `axios` . This exported module should be used instead of directly using `axios` to ensure the token is set. ## Usage + ```javascript import axios from './lib/utils/axios_utils'; diff --git a/doc/development/fe_guide/style_guide_scss.md b/doc/development/fe_guide/style_guide_scss.md index f895cfd36ac..95c4a094c04 100644 --- a/doc/development/fe_guide/style_guide_scss.md +++ b/doc/development/fe_guide/style_guide_scss.md @@ -212,6 +212,7 @@ selectors are intended for use only with JavaScript to allow for removal or renaming without breaking styling. ### IDs + Don't use ID selectors in CSS. ```scss diff --git a/doc/development/module_with_instance_variables.md b/doc/development/module_with_instance_variables.md index 7bdfa04fc57..443eee0b62c 100644 --- a/doc/development/module_with_instance_variables.md +++ b/doc/development/module_with_instance_variables.md @@ -1,6 +1,6 @@ -## Modules with instance variables could be considered harmful +# Modules with instance variables could be considered harmful -### Background +## Background Rails somehow encourages people using modules and instance variables everywhere. For example, using instance variables in the controllers, @@ -9,7 +9,7 @@ helpers, and views. They're also encouraging the use of saving everything in a giant, single object, and people could access everything in that one giant object. -### The problems +## The problems Of course this is convenient to develop, because we just have everything within reach. However this has a number of downsides when that chosen object @@ -24,7 +24,7 @@ manipulated from 3 different modules. It's hard to track when those variables start giving us troubles. We don't know which module would suddenly change one of the variables. Everything could touch anything. -### Similar concerns +## Similar concerns People are saying multiple inheritance is bad. Mixing multiple modules with multiple instance variables scattering everywhere suffer from the same issue. @@ -40,7 +40,7 @@ Note that `included` doesn't solve the whole issue. They define the dependencies, but they still allow each modules to talk implicitly via the instance variables in the final giant object, and that's where the problem is. -### Solutions +## Solutions We should split the giant object into multiple objects, and they communicate with each other with the API, i.e. public methods. In short, composition over @@ -53,7 +53,7 @@ With clearly defined API, this would make things less coupled and much easier to debug and track, and much more extensible for other objects to use, because they communicate in a clear way, rather than implicit dependencies. -### Acceptable use +## Acceptable use However, it's not always bad to use instance variables in a module, as long as it's contained in the same module; that is, no other modules or @@ -74,7 +74,7 @@ Unfortunately it's not easy to code more complex rules into the cop, so we rely on people's best judgement. If we could find another good pattern we could easily add to the cop, we should do it. -### How to rewrite and avoid disabling this cop +## How to rewrite and avoid disabling this cop Even if we could just disable the cop, we should avoid doing so. Some code could be easily rewritten in simple form. Consider this acceptable method: @@ -181,7 +181,7 @@ rather than whatever includes the module, and those modules which were also included, making it much easier to track down any issues, and reducing the chance of having name conflicts. -### How to disable this cop +## How to disable this cop Put the disabling comment right after your code in the same line: @@ -210,14 +210,14 @@ end Note that you need to enable it at some point, otherwise everything below won't be checked. -### Things we might need to ignore right now +## Things we might need to ignore right now Because of the way Rails helpers and mailers work, we might not be able to avoid the use of instance variables there. For those cases, we could ignore them at the moment. At least we're not going to share those modules with other random objects, so they're still somewhat isolated. -### Instance variables in views +## Instance variables in views They're bad because we can't easily tell who's using the instance variables (from controller's point of view) and where we set them up (from partials' diff --git a/doc/development/new_fe_guide/development/accessibility.md b/doc/development/new_fe_guide/development/accessibility.md index 81a29170129..ae5c4c6a6cc 100644 --- a/doc/development/new_fe_guide/development/accessibility.md +++ b/doc/development/new_fe_guide/development/accessibility.md @@ -1,17 +1,21 @@ # Accessiblity + Using semantic HTML plays a key role when it comes to accessibility. ## Accessible Rich Internet Applications - ARIA + WAI-ARIA, the Accessible Rich Internet Applications specification, defines a way to make Web content and Web applications more accessible to people with disabilities. > Note: It is [recommended][using-aria] to use semantic elements as the primary method to achieve accessibility rather than adding aria attributes. Adding aria attributes should be seen as a secondary method for creating accessible elements. ### Role + The `role` attribute describes the role the element plays in the context of the document. Check the list of WAI-ARIA roles [here][roles] ## Icons + When using icons or images that aren't absolutely needed to understand the context, we should use `aria-hidden="true"`. On the other hand, if an icon is crucial to understand the context we should do one of the following: @@ -20,6 +24,7 @@ On the other hand, if an icon is crucial to understand the context we should do 1. Use `aria-labelledby` to point to an element that contains the explanation for that icon ## Form inputs + In forms we should use the `for` attribute in the label statement: ``` diff --git a/doc/development/new_fe_guide/style/index.md b/doc/development/new_fe_guide/style/index.md index 335d9e66240..f073dc56f1f 100644 --- a/doc/development/new_fe_guide/style/index.md +++ b/doc/development/new_fe_guide/style/index.md @@ -8,7 +8,7 @@ ## [Vue style guide](vue.md) -# Tooling +## Tooling ## [Prettier](prettier.md) diff --git a/doc/development/verifying_database_capabilities.md b/doc/development/verifying_database_capabilities.md index 661ab9cef1a..ccec6f7d719 100644 --- a/doc/development/verifying_database_capabilities.md +++ b/doc/development/verifying_database_capabilities.md @@ -25,7 +25,7 @@ else end ``` -# Read-only database +## Read-only database The database can be used in read-only mode. In this case we have to make sure all GET requests don't attempt any write operations to the diff --git a/doc/user/admin_area/settings/usage_statistics.md b/doc/user/admin_area/settings/usage_statistics.md index f698e0a1608..516600c9d99 100644 --- a/doc/user/admin_area/settings/usage_statistics.md +++ b/doc/user/admin_area/settings/usage_statistics.md @@ -52,8 +52,8 @@ You can view the exact JSON payload in the administration panel. To view the pay 1. Expand **Settings** in the left sidebar and click on **Metrics and profiling**. 1. Expand **Usage statistics** and click on the **Preview payload** button. -You can see how [the usage ping data maps to different stages of the product](https://gitlab.com/gitlab-data/analytics/blob/master/transform/snowflake-dbt/data/ping_metrics_to_stage_mapping_data.csv). - +You can see how [the usage ping data maps to different stages of the product](https://gitlab.com/gitlab-data/analytics/blob/master/transform/snowflake-dbt/data/ping_metrics_to_stage_mapping_data.csv). + ### Deactivate the usage ping The usage ping is opt-out. If you want to deactivate this feature, go to diff --git a/doc/user/project/integrations/prometheus_library/haproxy.md b/doc/user/project/integrations/prometheus_library/haproxy.md index 6be8fc82431..90a725f271b 100644 --- a/doc/user/project/integrations/prometheus_library/haproxy.md +++ b/doc/user/project/integrations/prometheus_library/haproxy.md @@ -1,4 +1,5 @@ # Monitoring HAProxy + > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12621) in GitLab 9.4 GitLab has support for automatically detecting and monitoring HAProxy. This is provided by leveraging the [HAProxy Exporter](https://github.com/prometheus/haproxy_exporter), which translates HAProxy statistics into a Prometheus readable form. -- cgit v1.2.1 From 9a5ffe4057ddf8ec3a90975f49a56cd509db6bf8 Mon Sep 17 00:00:00 2001 From: Evan Read Date: Wed, 24 Jul 2019 13:25:55 +0000 Subject: Improve the Auto DevOps topic a bit --- doc/topics/autodevops/index.md | 103 +++++++++++----------- doc/user/project/clusters/eks_and_gitlab/index.md | 2 +- 2 files changed, 53 insertions(+), 52 deletions(-) diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 30b9e88ade8..7e1cf03908d 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -1,6 +1,7 @@ # Auto DevOps -> [Introduced][ce-37115] in GitLab 10.0. Generally available on GitLab 11.0. +> - [Introduced][ce-37115] in GitLab 10.0. +> - Generally available on GitLab 11.0. Auto DevOps provides pre-defined CI/CD configuration which allows you to automatically detect, build, test, deploy, and monitor your applications. Leveraging CI/CD best practices and tools, Auto DevOps aims @@ -43,16 +44,16 @@ platform or a Platform as a Service (PaaS). It takes inspiration from the innovative work done by [Heroku](https://www.heroku.com/) and goes beyond it in multiple ways: -1. Auto DevOps works with any Kubernetes cluster; you're not limited to running - on GitLab's infrastructure. (Note that many features also work without Kubernetes.) -1. There is no additional cost (no markup on the infrastructure costs), and you - can use a self-hosted Kubernetes cluster or Containers as a Service on any - public cloud (for example, [Google Kubernetes Engine](https://cloud.google.com/kubernetes-engine/)). -1. Auto DevOps has more features including security testing, performance testing, - and code quality testing. -1. Auto DevOps offers an incremental graduation path. If you need advanced customizations, - you can start modifying the templates without having to start over on a - completely different platform. Review the [customizing](#customizing) section for more information. +- Auto DevOps works with any Kubernetes cluster; you're not limited to running + on GitLab's infrastructure. (Note that many features also work without Kubernetes). +- There is no additional cost (no markup on the infrastructure costs), and you + can use a self-hosted Kubernetes cluster or Containers as a Service on any + public cloud (for example, [Google Kubernetes Engine](https://cloud.google.com/kubernetes-engine/)). +- Auto DevOps has more features including security testing, performance testing, + and code quality testing. +- Auto DevOps offers an incremental graduation path. If you need advanced customizations, + you can start modifying the templates without having to start over on a + completely different platform. Review the [customizing](#customizing) section for more information. ## Features @@ -90,38 +91,38 @@ For an overview on the creation of Auto DevOps, read the blog post [From 2/3 of To make full use of Auto DevOps, you will need: -1. **GitLab Runner** (needed for all stages) - Your Runner needs to be - configured to be able to run Docker. Generally this means using the - [Docker](https://docs.gitlab.com/runner/executors/docker.html) or [Kubernetes - executor](https://docs.gitlab.com/runner/executors/kubernetes.html), with - [privileged mode enabled](https://docs.gitlab.com/runner/executors/docker.html#use-docker-in-docker-with-privileged-mode). - The Runners do not need to be installed in the Kubernetes cluster, but the - Kubernetes executor is easy to use and is automatically autoscaling. - Docker-based Runners can be configured to autoscale as well, using [Docker - Machine](https://docs.gitlab.com/runner/install/autoscaling.html). Runners - should be registered as [shared Runners](../../ci/runners/README.md#registering-a-shared-runner) - for the entire GitLab instance, or [specific Runners](../../ci/runners/README.md#registering-a-specific-runner) - that are assigned to specific projects. -1. **Base domain** (needed for Auto Review Apps and Auto Deploy) - You will need - a domain configured with wildcard DNS which is going to be used by all of your - Auto DevOps applications. [Read the specifics](#auto-devops-base-domain). -1. **Kubernetes** (needed for Auto Review Apps, Auto Deploy, and Auto Monitoring) - - To enable deployments, you will need Kubernetes 1.5+. You need a [Kubernetes cluster][kubernetes-clusters] - for the project, or a Kubernetes [default service template](../../user/project/integrations/services_templates.md) - for the entire GitLab installation. - 1. **A load balancer** - You can use NGINX ingress by deploying it to your - Kubernetes cluster using the - [`nginx-ingress`](https://github.com/kubernetes/charts/tree/master/stable/nginx-ingress) - Helm chart. -1. **Prometheus** (needed for Auto Monitoring) - To enable Auto Monitoring, you - will need Prometheus installed somewhere (inside or outside your cluster) and - configured to scrape your Kubernetes cluster. To get response metrics - (in addition to system metrics), you need to - [configure Prometheus to monitor NGINX](../../user/project/integrations/prometheus_library/nginx_ingress.md#configuring-nginx-ingress-monitoring). - The [Prometheus service](../../user/project/integrations/prometheus.md) - integration needs to be enabled for the project, or enabled as a - [default service template](../../user/project/integrations/services_templates.md) - for the entire GitLab installation. +- **GitLab Runner** (needed for all stages) - Your Runner needs to be + configured to be able to run Docker. Generally this means using the + [Docker](https://docs.gitlab.com/runner/executors/docker.html) or [Kubernetes + executor](https://docs.gitlab.com/runner/executors/kubernetes.html), with + [privileged mode enabled](https://docs.gitlab.com/runner/executors/docker.html#use-docker-in-docker-with-privileged-mode). + The Runners do not need to be installed in the Kubernetes cluster, but the + Kubernetes executor is easy to use and is automatically autoscaling. + Docker-based Runners can be configured to autoscale as well, using [Docker + Machine](https://docs.gitlab.com/runner/install/autoscaling.html). Runners + should be registered as [shared Runners](../../ci/runners/README.md#registering-a-shared-runner) + for the entire GitLab instance, or [specific Runners](../../ci/runners/README.md#registering-a-specific-runner) + that are assigned to specific projects. +- **Base domain** (needed for Auto Review Apps and Auto Deploy) - You will need + a domain configured with wildcard DNS which is going to be used by all of your + Auto DevOps applications. [Read the specifics](#auto-devops-base-domain). +- **Kubernetes** (needed for Auto Review Apps, Auto Deploy, and Auto Monitoring) - + To enable deployments, you will need Kubernetes 1.5+. You need a [Kubernetes cluster][kubernetes-clusters] + for the project, or a Kubernetes [default service template](../../user/project/integrations/services_templates.md) + for the entire GitLab installation. + - **A load balancer** - You can use NGINX ingress by deploying it to your + Kubernetes cluster using the + [`nginx-ingress`](https://github.com/kubernetes/charts/tree/master/stable/nginx-ingress) + Helm chart. +- **Prometheus** (needed for Auto Monitoring) - To enable Auto Monitoring, you + will need Prometheus installed somewhere (inside or outside your cluster) and + configured to scrape your Kubernetes cluster. To get response metrics + (in addition to system metrics), you need to + [configure Prometheus to monitor NGINX](../../user/project/integrations/prometheus_library/nginx_ingress.md#configuring-nginx-ingress-monitoring). + The [Prometheus service](../../user/project/integrations/prometheus.md) + integration needs to be enabled for the project, or enabled as a + [default service template](../../user/project/integrations/services_templates.md) + for the entire GitLab installation. If you do not have Kubernetes or Prometheus installed, then Auto Review Apps, Auto Deploy, and Auto Monitoring will be silently skipped. @@ -147,7 +148,7 @@ NOTE: **Note** A wildcard DNS A record matching the base domain(s) is required, for example, given a base domain of `example.com`, you'd need a DNS entry like: -``` +```text *.example.com 3600 A 1.2.3.4 ``` @@ -224,7 +225,7 @@ full use of Auto DevOps are available. If this is your fist time, we recommend y GitLab.com users can enable/disable Auto DevOps at the project-level only. Self-managed users can enable/disable Auto DevOps at the project-level, group-level or instance-level. -### Enabling/disabling Auto DevOps at the instance-level (Administrators only) +### At the instance level (Administrators only) Even when disabled at the instance level, group owners and project maintainers can still enable Auto DevOps at the group and project level, respectively. @@ -234,7 +235,7 @@ Auto DevOps at the group and project level, respectively. 1. If enabling, optionally set up the Auto DevOps [base domain](#auto-devops-base-domain) which will be used for Auto Deploy and Auto Review Apps. 1. Click **Save changes** for the changes to take effect. -### Enabling/disabling Auto DevOps at the group-level +### At the group level > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/52447) in GitLab 11.10. @@ -250,7 +251,7 @@ When enabling or disabling Auto DevOps at group-level, group configuration will the subgroups and projects inside that group, unless Auto DevOps is specifically enabled or disabled on the subgroup or project. -### Enabling/disabling Auto DevOps at the project-level +### At the project level If enabling, check that your project doesn't have a `.gitlab-ci.yml`, or if one exists, remove it. @@ -263,7 +264,7 @@ If enabling, check that your project doesn't have a `.gitlab-ci.yml`, or if one When the feature has been enabled, an Auto DevOps pipeline is triggered on the default branch. -### Feature flag to enable for a percentage of projects +### Enable for a percentage of projects There is also a feature flag to enable Auto DevOps by default to your chosen percentage of projects. @@ -371,7 +372,7 @@ Any differences between the source and target branches are also Static Application Security Testing (SAST) uses the [SAST Docker image](https://gitlab.com/gitlab-org/security-products/sast) to run static analysis on the current code and checks for potential security issues. The -the Auto SAST stage will be skipped on licenses other than Ultimate and requires GitLab Runner 11.5 or above. +Auto SAST stage will be skipped on licenses other than Ultimate and requires GitLab Runner 11.5 or above. Once the report is created, it's uploaded as an artifact which you can later download and check out. @@ -488,7 +489,7 @@ Any security warnings are also shown in the merge request widget. Read how Auto Browser Performance Testing utilizes the [Sitespeed.io container](https://hub.docker.com/r/sitespeedio/sitespeed.io/) to measure the performance of a web page. A JSON report is created and uploaded as an artifact, which includes the overall performance score for each page. By default, the root page of Review and Production environments will be tested. If you would like to add additional URL's to test, simply add the paths to a file named `.gitlab-urls.txt` in the root directory, one per line. For example: -``` +```text / /features /direction @@ -1009,7 +1010,7 @@ multi-buildpack does not. As of GitLab 10.0, the supported buildpacks are: -``` +```text - heroku-buildpack-multi v1.0.0 - heroku-buildpack-ruby v168 - heroku-buildpack-nodejs v99 diff --git a/doc/user/project/clusters/eks_and_gitlab/index.md b/doc/user/project/clusters/eks_and_gitlab/index.md index 55a9fbabf98..28f3420de35 100644 --- a/doc/user/project/clusters/eks_and_gitlab/index.md +++ b/doc/user/project/clusters/eks_and_gitlab/index.md @@ -253,7 +253,7 @@ With RBAC disabled and services deployed, [Auto DevOps](../../../../topics/autodevops/index.md) can now be leveraged to build, test, and deploy the app. -[Enable Auto DevOps](../../../../topics/autodevops/index.md#enablingdisabling-auto-devops-at-the-project-level) +[Enable Auto DevOps](../../../../topics/autodevops/index.md#at-the-project-level) if not already enabled. If a wildcard DNS entry was created resolving to the Load Balancer, enter it in the `domain` field under the Auto DevOps settings. Otherwise, the deployed app will not be externally available outside of the cluster. -- cgit v1.2.1 From b8683c266d623b6e51b0254ce38ad7a8edece7f4 Mon Sep 17 00:00:00 2001 From: Evan Read Date: Wed, 24 Jul 2019 13:28:46 +0000 Subject: Better information on blocking users --- doc/user/abuse_reports.md | 22 +++++--- doc/user/admin_area/abuse_reports.md | 52 +++++++++++++++---- doc/user/profile/account/delete_account.md | 82 +++++++++++++++++++++--------- 3 files changed, 115 insertions(+), 41 deletions(-) diff --git a/doc/user/abuse_reports.md b/doc/user/abuse_reports.md index 41ee7e62b2c..e6c86cc8f2e 100644 --- a/doc/user/abuse_reports.md +++ b/doc/user/abuse_reports.md @@ -1,6 +1,12 @@ # Abuse reports -Report abuse from users to GitLab administrators. +You can report abuse from other GitLab users to GitLab administrators. + +A GitLab administrator [can then choose](admin_area/abuse_reports.md) to: + +- Remove the user, which deletes them from the instance. +- Block the user, which denies them access to the instance. +- Or remove the report, which retains the users access to the instance. You can report a user through their: @@ -12,7 +18,8 @@ You can report a user through their: To report abuse from a user's profile page: -1. Click on the exclamation point report abuse button at the top right of the user's profile. +1. Click on the exclamation point report abuse button at the top right of the + user's profile. 1. Complete an abuse report. 1. Click the **Send report** button. @@ -26,15 +33,18 @@ To report abuse from a user's comment: 1. Click the **Send report** button. NOTE: **Note:** -A URL to the reported user's comment will be -pre-filled in the abuse report's **Message** field. +A URL to the reported user's comment will be pre-filled in the abuse report's +**Message** field. ## Reporting abuse through a user's issue or merge request The **Report abuse** button is displayed at the top right of the issue or merge request: -- When **Report abuse** is selected from the menu that appears when the **Close issue** or **Close merge request** button is clicked, for users that have permission to close the issue or merge request. -- When viewing the issue or merge request, for users that don't have permission to close the issue or merge request. +- When **Report abuse** is selected from the menu that appears when the + **Close issue** or **Close merge request** button is clicked, for users that + have permission to close the issue or merge request. +- When viewing the issue or merge request, for users that don't have permission + to close the issue or merge request. With the **Report abuse** button displayed, to submit an abuse report: diff --git a/doc/user/admin_area/abuse_reports.md b/doc/user/admin_area/abuse_reports.md index 01c2d9607f5..8088c33fc9c 100644 --- a/doc/user/admin_area/abuse_reports.md +++ b/doc/user/admin_area/abuse_reports.md @@ -2,30 +2,60 @@ View and resolve abuse reports from GitLab users. -Admins can view abuse reports in the admin area and are able to -resolve the reports by removing the reported user, blocking the reported user, or removing the report. +GitLab administrators can view and [resolve](#resolving-abuse-reports) abuse +reports in the Admin Area. ## Reporting abuse -To find out more about reporting abuse, see [abuse reports user documentation](../abuse_reports.md). +To find out more about reporting abuse, see [abuse reports user +documentation](../abuse_reports.md). ## Resolving abuse reports -To access abuse reports, go to **Admin area > Abuse Reports**. +To access abuse reports, go to **Admin Area > Abuse Reports**. There are 3 ways to resolve an abuse report, with a button for each method: -- Remove user & report: [Deletes the reported user](../profile/account/delete_account.md) from the instance and removes the abuse report from the list. -- Block user: Blocks the reported user from the instance and does not remove the abuse report from the list. -- Remove report: Removes the abuse report from the list and does not restrict the access for the reported user. +- Remove user & report. This will: + - [Delete the reported user](../profile/account/delete_account.md) from the + instance. + - Remove the abuse report from the list. +- [Block user](#blocking-users). +- Remove report. This will: + - Remove the abuse report from the list. + - Remove access restrictions for the reported user. + +The following is an example of the **Abuse Reports** page: ![abuse-reports-page-image](img/abuse_reports_page.png) -## Blocked users +### Blocking users + +A blocked user cannot log in or access any repositories, but all of their data +remains. + +Blocking a user: + +- Leaves them in the abuse report list. +- Changes the **Block user** button to a disabled **Already blocked** button. -Blocking a user will not remove the abuse report from the list. +The user will be notified with the +[following message](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/workers/email_receiver_worker.rb#L38): -Instead, the block button will be disabled and explain that the user is "Already blocked". -You are still able to remove the user and/or report if necessary. +```text +Your account has been blocked. If you believe this is in error, contact a staff member. +``` + +After blocking, you can still either: + +- Remove the user and report if necessary. +- Remove the report. + +The following is an example of a blocked user listed on the **Abuse Reports** +page: ![abuse-report-blocked-user-image](img/abuse_report_blocked_user.png) + +NOTE: **Note:** +Users can be [blocked](../../api/users.md#block-user) and +[unblocked](../../api/users.md#unblock-user) using the GitLab API. diff --git a/doc/user/profile/account/delete_account.md b/doc/user/profile/account/delete_account.md index 304a7984191..cbee79de493 100644 --- a/doc/user/profile/account/delete_account.md +++ b/doc/user/profile/account/delete_account.md @@ -1,37 +1,71 @@ -# Deleting a User Account +# Deleting a User account + +Users can be deleted from a GitLab instance, either by: + +- The user themselves. +- An administrator. NOTE: **Note:** Deleting a user will delete all projects in that user namespace. -- As a user, you can delete your own account by navigating to **Settings** > **Account** and selecting **Delete account** -- As an admin, you can delete a user account by navigating to the **Admin Area**, selecting the **Users** tab, selecting a user, and clicking on **Delete user** +## As a user + +As a user, you can delete your own account by: + +1. Clicking on your avatar. +1. Navigating to **Settings > Account**. +1. Selecting **Delete account**. + +## As an administrator + +As an administrator, you can delete a user account by: + +1. Navigating to **Admin Area > Overview > Users**. +1. Selecting a user. +1. Under the **Account** tab, clicking: + - **Delete user** to delete only the user but maintaining their + [associated records](#associated-records). + - **Delete user and contributions** to delete the user and + their associated records. + +### Blocking a user + +In addition to blocking a user +[via an abuse report](../../admin_area/abuse_reports.md#blocking-users), +a user can be blocked directly from the Admin area. To do this: + +1. Navigate to **Admin Area > Overview > Users**. +1. Selecting a user. +1. Under the **Account** tab, click **Block user**. ## Associated Records -> Introduced for issues in [GitLab 9.0][ce-7393], and for merge requests, award - emoji, notes, and abuse reports in [GitLab 9.1][ce-10467]. - Hard deletion from abuse reports and spam logs was introduced in - [GitLab 9.1][ce-10273], and from the API in [GitLab 9.3][ce-11853]. +> - Introduced for issues in +> [GitLab 9.0](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7393). +> - Introduced for merge requests, award emoji, notes, and abuse reports in +> [GitLab 9.1](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10467). +> - Hard deletion from abuse reports and spam logs was introduced in +> [GitLab 9.1](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10273), +> and from the API in +> [GitLab 9.3](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11853). When a user account is deleted, not all associated records are deleted with it. Here's a list of things that will **not** be deleted: -- Issues that the user created -- Merge requests that the user created -- Notes that the user created -- Abuse reports that the user reported -- Award emoji that the user created +- Issues that the user created. +- Merge requests that the user created. +- Notes that the user created. +- Abuse reports that the user reported. +- Award emoji that the user created. Instead of being deleted, these records will be moved to a system-wide -user with the username "Ghost User", whose sole purpose is to act as a container for such records. Any commits made by a deleted user will still display the username of the original user. - -When a user is deleted from an [abuse report](../../admin_area/abuse_reports.md) or spam log, these associated -records are not ghosted and will be removed, along with any groups the user -is a sole owner of. Administrators can also request this behaviour when -deleting users from the [API](../../../api/users.md#user-deletion) or the -admin area. - -[ce-7393]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7393 -[ce-10273]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10273 -[ce-10467]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10467 -[ce-11853]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11853 +user with the username "Ghost User", whose sole purpose is to act as a container +for such records. Any commits made by a deleted user will still display the +username of the original user. + +When a user is deleted from an [abuse report](../../admin_area/abuse_reports.md) +or spam log, these associated records are not ghosted and will be removed, along +with any groups the user is a sole owner of. + +Administrators can also request this behavior when deleting users from the +[API](../../../api/users.md#user-deletion) or the Admin Area. -- cgit v1.2.1 From a0adccd210e5dea3681a66a439502e075098e062 Mon Sep 17 00:00:00 2001 From: Evan Read Date: Wed, 24 Jul 2019 13:32:15 +0000 Subject: Fix some errors in Markdown files --- doc/administration/geo/replication/configuration.md | 4 ++-- doc/administration/geo/replication/updating_the_geo_nodes.md | 2 +- doc/api/README.md | 2 +- doc/topics/autodevops/index.md | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/administration/geo/replication/configuration.md b/doc/administration/geo/replication/configuration.md index 0e11dffa0d6..fd076bb79d8 100644 --- a/doc/administration/geo/replication/configuration.md +++ b/doc/administration/geo/replication/configuration.md @@ -17,7 +17,7 @@ You are encouraged to first read through all the steps before executing them in your testing/production environment. NOTE: **Note:** -**Do not** set up any custom authentication for the **secondary** nodes. This will be handled by the **primary** node. +**Do not** set up any custom authentication for the **secondary** nodes. This will be handled by the **primary** node. Any change that requires access to the **Admin Area** needs to be done in the **primary** node because the **secondary** node is a read-only replica. @@ -242,7 +242,7 @@ node's Geo Nodes dashboard in your browser. ![Geo dashboard](img/geo_node_dashboard.png) If your installation isn't working properly, check the -[troubleshooting document]. +[troubleshooting document](troubleshooting.md). The two most obvious issues that can become apparent in the dashboard are: diff --git a/doc/administration/geo/replication/updating_the_geo_nodes.md b/doc/administration/geo/replication/updating_the_geo_nodes.md index 166ee94eca4..550b3b07a95 100644 --- a/doc/administration/geo/replication/updating_the_geo_nodes.md +++ b/doc/administration/geo/replication/updating_the_geo_nodes.md @@ -251,7 +251,7 @@ Omnibus is the following: 1. Check the steps about defining `postgresql['sql_user_password']`, `gitlab_rails['db_password']`. 1. Make sure `postgresql['max_replication_slots']` matches the number of **secondary** Geo nodes locations. 1. Install GitLab on the **secondary** server. -1. Re-run the [database replication process][database-replication]. +1. Re-run the [database replication process](database.md#step-3-initiate-the-replication-process). ## Special update notes for 9.0.x diff --git a/doc/api/README.md b/doc/api/README.md index f9cc1a09870..91bc8ca8aab 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -29,7 +29,7 @@ The following API resources are available in the project context: | [Commits](commits.md) | `/projects/:id/repository/commits`, `/projects/:id/statuses` | | [Container Registry](container_registry.md) | `/projects/:id/registry/repositories` | | [Custom attributes](custom_attributes.md) | `/projects/:id/custom_attributes` (also available for groups and users) | -| [Dependencies](dependencies.md) **[ULTIMATE]** | `/projects/:id/dependencies` +| [Dependencies](dependencies.md) **(ULTIMATE)** | `/projects/:id/dependencies` | [Deploy keys](deploy_keys.md) | `/projects/:id/deploy_keys` (also available standalone) | | [Deployments](deployments.md) | `/projects/:id/deployments` | | [Discussions](discussions.md) (threaded comments) | `/projects/:id/issues/.../discussions`, `/projects/:id/snippets/.../discussions`, `/projects/:id/merge_requests/.../discussions`, `/projects/:id/commits/.../discussions` (also available for groups) | diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 7e1cf03908d..c416dbac492 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -750,7 +750,7 @@ also be customized, and you can easily use a [custom buildpack](#custom-buildpac | `DB_MIGRATE` | From GitLab 11.4, this variable can be used to specify the command to run to migrate the application's PostgreSQL database. It runs inside the application pod. | | `STAGING_ENABLED` | From GitLab 10.8, this variable can be used to define a [deploy policy for staging and production environments](#deploy-policy-for-staging-and-production-environments). | | `CANARY_ENABLED` | From GitLab 11.0, this variable can be used to define a [deploy policy for canary environments](#deploy-policy-for-canary-environments-premium). | -| `INCREMENTAL_ROLLOUT_MODE`| From GitLab 11.4, this variable, if present, can be used to enable an [incremental rollout](#incremental-rollout-to-production-premium) of your application for the production environment.
Set to:
  • `manual`, for manual deployment jobs.
  • `timed`, for automatic rollout deployments with a 5 minute delay each one.
| +| `INCREMENTAL_ROLLOUT_MODE`| From GitLab 11.4, this variable, if present, can be used to enable an [incremental rollout](#incremental-rollout-to-production-premium) of your application for the production environment. Set to `manual` for manual deployment jobs or `timed` for automatic rollout deployments with a 5 minute delay each one. | | `TEST_DISABLED` | From GitLab 11.0, this variable can be used to disable the `test` job. If the variable is present, the job will not be created. | | `CODE_QUALITY_DISABLED` | From GitLab 11.0, this variable can be used to disable the `codequality` job. If the variable is present, the job will not be created. | | `LICENSE_MANAGEMENT_DISABLED` | From GitLab 11.0, this variable can be used to disable the `license_management` job. If the variable is present, the job will not be created. | -- cgit v1.2.1 From 68d1a9a306ae927013b194d3ffb8602e490d6dca Mon Sep 17 00:00:00 2001 From: Brandon Labuschagne Date: Wed, 24 Jul 2019 13:59:23 +0000 Subject: Add projectIds to CA service --- app/assets/javascripts/cycle_analytics/cycle_analytics_service.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/cycle_analytics/cycle_analytics_service.js b/app/assets/javascripts/cycle_analytics/cycle_analytics_service.js index a0426301a0a..babbfe93082 100644 --- a/app/assets/javascripts/cycle_analytics/cycle_analytics_service.js +++ b/app/assets/javascripts/cycle_analytics/cycle_analytics_service.js @@ -8,22 +8,26 @@ export default class CycleAnalyticsService { } fetchCycleAnalyticsData(options = { startDate: 30 }) { + const { startDate, projectIds } = options; + return this.axios .get('', { params: { - 'cycle_analytics[start_date]': options.startDate, + 'cycle_analytics[start_date]': startDate, + 'cycle_analytics[project_ids]': projectIds, }, }) .then(x => x.data); } fetchStageData(options) { - const { stage, startDate } = options; + const { stage, startDate, projectIds } = options; return this.axios .get(`events/${stage.name}.json`, { params: { 'cycle_analytics[start_date]': startDate, + 'cycle_analytics[project_ids]': projectIds, }, }) .then(x => x.data); -- cgit v1.2.1 From 8d1e97fc3b9af28d2a34d2b16239e52d3b5d0303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Tue, 23 Jul 2019 11:28:22 +0200 Subject: Optimise import performance - Fix `O(n)` complexity of `append_or_update_attribute`, we append objects to an array and re-save project - Remove the usage of `keys.include?` as it performs `O(n)` search, instead use `.has_key?` - Remove the usage of `.keys.first` as it performs a copy of all keys, instead use `.first.first` --- app/models/project.rb | 32 ++++++++++------------ .../unreleased/optimise-import-performance.yml | 5 ++++ lib/gitlab/import_export/attributes_finder.rb | 2 +- lib/gitlab/import_export/json_hash_builder.rb | 2 +- lib/gitlab/import_export/members_mapper.rb | 2 +- lib/gitlab/import_export/relation_factory.rb | 2 +- spec/controllers/user_callouts_controller_spec.rb | 4 +-- spec/lib/gitlab/import_export/project.group.json | 6 ++-- .../import_export/project_tree_restorer_spec.rb | 6 ++-- spec/models/project_spec.rb | 5 +--- 10 files changed, 32 insertions(+), 34 deletions(-) create mode 100644 changelogs/unreleased/optimise-import-performance.yml diff --git a/app/models/project.rb b/app/models/project.rb index ece7507e55c..08221642295 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1854,16 +1854,24 @@ class Project < ApplicationRecord end def append_or_update_attribute(name, value) - old_values = public_send(name.to_s) # rubocop:disable GitlabSecurity/PublicSend + if Project.reflect_on_association(name).try(:macro) == :has_many + # if this is 1-to-N relation, update the parent object + value.each do |item| + item.update!( + Project.reflect_on_association(name).foreign_key => id) + end + + # force to drop relation cache + public_send(name).reset # rubocop:disable GitlabSecurity/PublicSend - if Project.reflect_on_association(name).try(:macro) == :has_many && old_values.any? - update_attribute(name, old_values + value) + # succeeded + true else + # if this is another relation or attribute, update just object update_attribute(name, value) end - - rescue ActiveRecord::RecordNotSaved => e - handle_update_attribute_error(e, value) + rescue ActiveRecord::RecordInvalid => e + raise e, "Failed to set #{name}: #{e.message}" end # Tries to set repository as read_only, checking for existing Git transfers in progress beforehand @@ -2252,18 +2260,6 @@ class Project < ApplicationRecord ContainerRepository.build_root_repository(self).has_tags? end - def handle_update_attribute_error(ex, value) - if ex.message.start_with?('Failed to replace') - if value.respond_to?(:each) - invalid = value.detect(&:invalid?) - - raise ex, ([ex.message] + invalid.errors.full_messages).join(' ') if invalid - end - end - - raise ex - end - def fetch_branch_allows_collaboration(user, branch_name = nil) return false unless user diff --git a/changelogs/unreleased/optimise-import-performance.yml b/changelogs/unreleased/optimise-import-performance.yml new file mode 100644 index 00000000000..c63f44d5109 --- /dev/null +++ b/changelogs/unreleased/optimise-import-performance.yml @@ -0,0 +1,5 @@ +--- +title: Optimise import performance +merge_request: 31045 +author: +type: performance diff --git a/lib/gitlab/import_export/attributes_finder.rb b/lib/gitlab/import_export/attributes_finder.rb index 409243e68a5..42cd94add79 100644 --- a/lib/gitlab/import_export/attributes_finder.rb +++ b/lib/gitlab/import_export/attributes_finder.rb @@ -45,7 +45,7 @@ module Gitlab end def key_from_hash(value) - value.is_a?(Hash) ? value.keys.first : value + value.is_a?(Hash) ? value.first.first : value end end end diff --git a/lib/gitlab/import_export/json_hash_builder.rb b/lib/gitlab/import_export/json_hash_builder.rb index b145f37c052..a92e3862361 100644 --- a/lib/gitlab/import_export/json_hash_builder.rb +++ b/lib/gitlab/import_export/json_hash_builder.rb @@ -27,7 +27,7 @@ module Gitlab # {:merge_requests=>[:merge_request_diff, :notes]} def process_model_objects(model_object_hash) json_config_hash = {} - current_key = model_object_hash.keys.first + current_key = model_object_hash.first.first model_object_hash.values.flatten.each do |model_object| @attributes_finder.parse(current_key) { |hash| json_config_hash[current_key] ||= hash } diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index a154de5419e..71e9064c5fd 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -35,7 +35,7 @@ module Gitlab end def include?(old_author_id) - map.keys.include?(old_author_id) && map[old_author_id] != default_user_id + map.has_key?(old_author_id) && map[old_author_id] != default_user_id end private diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 1b545b1d049..0be49e27acb 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -185,7 +185,7 @@ module Gitlab return unless EXISTING_OBJECT_CHECK.include?(@relation_name) return unless @relation_hash['group_id'] - @relation_hash['group_id'] = @project.group&.id + @relation_hash['group_id'] = @project.namespace_id end def reset_tokens! diff --git a/spec/controllers/user_callouts_controller_spec.rb b/spec/controllers/user_callouts_controller_spec.rb index babc93a83e5..07eaff2da09 100644 --- a/spec/controllers/user_callouts_controller_spec.rb +++ b/spec/controllers/user_callouts_controller_spec.rb @@ -13,7 +13,7 @@ describe UserCalloutsController do subject { post :create, params: { feature_name: feature_name }, format: :json } context 'with valid feature name' do - let(:feature_name) { UserCallout.feature_names.keys.first } + let(:feature_name) { UserCallout.feature_names.first.first } context 'when callout entry does not exist' do it 'creates a callout entry with dismissed state' do @@ -28,7 +28,7 @@ describe UserCalloutsController do end context 'when callout entry already exists' do - let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.keys.first, user: user) } + let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.first.first, user: user) } it 'returns success' do subject diff --git a/spec/lib/gitlab/import_export/project.group.json b/spec/lib/gitlab/import_export/project.group.json index 1a561e81e4a..66f5bb4c87b 100644 --- a/spec/lib/gitlab/import_export/project.group.json +++ b/spec/lib/gitlab/import_export/project.group.json @@ -19,7 +19,7 @@ "labels": [ { "id": 2, - "title": "project label", + "title": "A project label", "color": "#428bca", "project_id": 8, "created_at": "2016-07-22T08:55:44.161Z", @@ -105,7 +105,7 @@ "updated_at": "2017-08-15T18:37:40.795Z", "label": { "id": 6, - "title": "project label", + "title": "A project label", "color": "#A8D695", "project_id": null, "created_at": "2017-08-15T18:37:19.698Z", @@ -162,7 +162,7 @@ "updated_at": "2017-08-15T18:37:40.795Z", "label": { "id": 2, - "title": "project label", + "title": "A project label", "color": "#A8D695", "project_id": null, "created_at": "2017-08-15T18:37:19.698Z", diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 3b7de185cf1..b9f6595762b 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -272,7 +272,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end it 'has label priorities' do - expect(project.labels.first.priorities).not_to be_empty + expect(project.labels.find_by(title: 'A project label').priorities).not_to be_empty end it 'has milestones' do @@ -325,7 +325,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do it_behaves_like 'restores project correctly', issues: 1, - labels: 1, + labels: 2, milestones: 1, first_issue_labels: 1, services: 1 @@ -402,7 +402,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do it_behaves_like 'restores project successfully' it_behaves_like 'restores project correctly', issues: 2, - labels: 1, + labels: 2, milestones: 2, first_issue_labels: 1 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index bcb2da7eed2..da9e204d4ca 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3097,11 +3097,8 @@ describe Project do let(:project) { create(:project) } it 'shows full error updating an invalid MR' do - error_message = 'Failed to replace merge_requests because one or more of the new records could not be saved.'\ - ' Validate fork Source project is not a fork of the target project' - expect { project.append_or_update_attribute(:merge_requests, [create(:merge_request)]) } - .to raise_error(ActiveRecord::RecordNotSaved, error_message) + .to raise_error(ActiveRecord::RecordInvalid, /Failed to set merge_requests:/) end it 'updates the project successfully' do -- cgit v1.2.1 From 1902d9cc74a1dc2c87fdbb39a6cdbb67092cbb5a Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 9 Jul 2019 18:59:52 +0300 Subject: Backport of https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/14597 This is the first part of Docker Registry replication for secondary Geo node. --- app/models/container_repository.rb | 5 +++++ config/gitlab.yml.example | 9 ++++++++ config/initializers/1_settings.rb | 1 + ...eo_container_repository_updated_events_table.rb | 17 +++++++++++++++ .../20190611100202_add_index_to_geo_event_log.rb | 17 +++++++++++++++ ...64_add_foreign_keys_for_container_repository.rb | 25 ++++++++++++++++++++++ db/schema.rb | 9 ++++++++ 7 files changed, 83 insertions(+) create mode 100644 db/migrate/20190611100201_add_geo_container_repository_updated_events_table.rb create mode 100644 db/migrate/20190611100202_add_index_to_geo_event_log.rb create mode 100644 db/migrate/20190627122264_add_foreign_keys_for_container_repository.rb diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index 39e12ac2b06..facd81dde80 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -86,4 +86,9 @@ class ContainerRepository < ApplicationRecord def self.build_root_repository(project) self.new(project: project, name: '') end + + def self.find_by_path!(path) + self.find_by!(project: path.repository_project, + name: path.repository_name) + end end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 0e78980350f..dd53127ac2c 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -400,6 +400,15 @@ production: &base # path: shared/registry # issuer: gitlab-issuer + # Add notification settings if you plan to use Geo Replication for the registry + # notifications: + # - name: geo_event + # url: https://example.com/api/v4/container_registry_event/events + # timeout: 2s + # threshold: 5 + # backoff: 1s + # headers: + # Authorization: secret_phrase ## Error Reporting and Logging with Sentry sentry: diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 494c4dd1f93..32fec7c3d22 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -259,6 +259,7 @@ Settings.registry['key'] ||= nil Settings.registry['issuer'] ||= nil Settings.registry['host_port'] ||= [Settings.registry['host'], Settings.registry['port']].compact.join(':') Settings.registry['path'] = Settings.absolute(Settings.registry['path'] || File.join(Settings.shared['path'], 'registry')) +Settings.registry['notifications'] ||= [] # # Error Reporting and Logging with Sentry diff --git a/db/migrate/20190611100201_add_geo_container_repository_updated_events_table.rb b/db/migrate/20190611100201_add_geo_container_repository_updated_events_table.rb new file mode 100644 index 00000000000..8963e837e08 --- /dev/null +++ b/db/migrate/20190611100201_add_geo_container_repository_updated_events_table.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddGeoContainerRepositoryUpdatedEventsTable < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :geo_container_repository_updated_events, force: :cascade do |t| + t.integer :container_repository_id, null: false + + t.index :container_repository_id, name: :idx_geo_con_rep_updated_events_on_container_repository_id, using: :btree + end + + add_column :geo_event_log, :container_repository_updated_event_id, :bigint + end +end diff --git a/db/migrate/20190611100202_add_index_to_geo_event_log.rb b/db/migrate/20190611100202_add_index_to_geo_event_log.rb new file mode 100644 index 00000000000..c5c855fed61 --- /dev/null +++ b/db/migrate/20190611100202_add_index_to_geo_event_log.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToGeoEventLog < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :geo_event_log, :container_repository_updated_event_id + end + + def down + remove_concurrent_index(:geo_event_log, :container_repository_updated_event_id) + end +end diff --git a/db/migrate/20190627122264_add_foreign_keys_for_container_repository.rb b/db/migrate/20190627122264_add_foreign_keys_for_container_repository.rb new file mode 100644 index 00000000000..cf7fd03e9af --- /dev/null +++ b/db/migrate/20190627122264_add_foreign_keys_for_container_repository.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddForeignKeysForContainerRepository < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key(:geo_container_repository_updated_events, :container_repositories, column: :container_repository_id, on_delete: :cascade) + + add_concurrent_foreign_key(:geo_event_log, :geo_container_repository_updated_events, column: :container_repository_updated_event_id, on_delete: :cascade) + end + + def down + if foreign_key_exists?(:geo_container_repository_updated_events, :container_repositories) + remove_foreign_key(:geo_container_repository_updated_events, :container_repositories) + end + + if foreign_key_exists?(:geo_event_log, :geo_container_repository_updated_events) + remove_foreign_key(:geo_event_log, :geo_container_repository_updated_events) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index f42a4c600ea..0e03c2eb2ec 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1275,6 +1275,11 @@ ActiveRecord::Schema.define(version: 2019_07_15_114644) do t.string "key", null: false end + create_table "geo_container_repository_updated_events", force: :cascade do |t| + t.integer "container_repository_id", null: false + t.index ["container_repository_id"], name: "idx_geo_con_rep_updated_events_on_container_repository_id" + end + create_table "geo_event_log", force: :cascade do |t| t.datetime "created_at", null: false t.bigint "repository_updated_event_id" @@ -1289,7 +1294,9 @@ ActiveRecord::Schema.define(version: 2019_07_15_114644) do t.bigint "job_artifact_deleted_event_id" t.bigint "reset_checksum_event_id" t.bigint "cache_invalidation_event_id" + t.bigint "container_repository_updated_event_id" t.index ["cache_invalidation_event_id"], name: "index_geo_event_log_on_cache_invalidation_event_id", where: "(cache_invalidation_event_id IS NOT NULL)" + t.index ["container_repository_updated_event_id"], name: "index_geo_event_log_on_container_repository_updated_event_id" t.index ["hashed_storage_attachments_event_id"], name: "index_geo_event_log_on_hashed_storage_attachments_event_id", where: "(hashed_storage_attachments_event_id IS NOT NULL)" t.index ["hashed_storage_migrated_event_id"], name: "index_geo_event_log_on_hashed_storage_migrated_event_id", where: "(hashed_storage_migrated_event_id IS NOT NULL)" t.index ["job_artifact_deleted_event_id"], name: "index_geo_event_log_on_job_artifact_deleted_event_id", where: "(job_artifact_deleted_event_id IS NOT NULL)" @@ -3700,7 +3707,9 @@ ActiveRecord::Schema.define(version: 2019_07_15_114644) do add_foreign_key "fork_network_members", "projects", on_delete: :cascade add_foreign_key "fork_networks", "projects", column: "root_project_id", name: "fk_e7b436b2b5", on_delete: :nullify add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade + add_foreign_key "geo_container_repository_updated_events", "container_repositories", name: "fk_212c89c706", on_delete: :cascade add_foreign_key "geo_event_log", "geo_cache_invalidation_events", column: "cache_invalidation_event_id", name: "fk_42c3b54bed", on_delete: :cascade + add_foreign_key "geo_event_log", "geo_container_repository_updated_events", column: "container_repository_updated_event_id", name: "fk_6ada82d42a", on_delete: :cascade add_foreign_key "geo_event_log", "geo_hashed_storage_migrated_events", column: "hashed_storage_migrated_event_id", name: "fk_27548c6db3", on_delete: :cascade add_foreign_key "geo_event_log", "geo_job_artifact_deleted_events", column: "job_artifact_deleted_event_id", name: "fk_176d3fbb5d", on_delete: :cascade add_foreign_key "geo_event_log", "geo_lfs_object_deleted_events", column: "lfs_object_deleted_event_id", name: "fk_d5af95fcd9", on_delete: :cascade -- cgit v1.2.1 From dee72df589fae9e2c03af5539b24aa866c2da37c Mon Sep 17 00:00:00 2001 From: Evan Read Date: Wed, 24 Jul 2019 15:47:06 +0000 Subject: Followup edit of documentation --- doc/development/chaos_endpoints.md | 8 ++++---- doc/development/reusing_abstractions.md | 11 +++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/doc/development/chaos_endpoints.md b/doc/development/chaos_endpoints.md index eb6dde2d24e..961520db7d8 100644 --- a/doc/development/chaos_endpoints.md +++ b/doc/development/chaos_endpoints.md @@ -69,7 +69,7 @@ curl http://localhost:3000/-/chaos/leakmem?memory_mb=1024&duration_s=10&token=se This endpoint attempts to fully utilise a single core, at 100%, for the given period. -Depending on your rack server setup, your request may timeout after a predermined period (normally 60 seconds). +Depending on your rack server setup, your request may timeout after a predetermined period (normally 60 seconds). If you're using Unicorn, this is done by killing the worker process. ``` @@ -80,7 +80,7 @@ GET /-/chaos/cpu_spin?duration_s=50&async=true | Attribute | Type | Required | Description | | ------------ | ------- | -------- | --------------------------------------------------------------------- | -| `duration_s` | integer | no | Duration, in seconds, that the core will be utilised. Defaults to 30s | +| `duration_s` | integer | no | Duration, in seconds, that the core will be utilized. Defaults to 30s | | `async` | boolean | no | Set to true to consume CPU in a Sidekiq background worker process | ```bash @@ -93,7 +93,7 @@ curl http://localhost:3000/-/chaos/cpu_spin?duration_s=60&token=secret This endpoint attempts to fully utilise a single core, and interleave it with DB request, for the given period. This endpoint can be used to model yielding execution to another threads when running concurrently. -Depending on your rack server setup, your request may timeout after a predermined period (normally 60 seconds). +Depending on your rack server setup, your request may timeout after a predetermined period (normally 60 seconds). If you're using Unicorn, this is done by killing the worker process. ``` @@ -105,7 +105,7 @@ GET /-/chaos/db_spin?duration_s=50&async=true | Attribute | Type | Required | Description | | ------------ | ------- | -------- | --------------------------------------------------------------------------- | | `interval_s` | float | no | Interval, in seconds, for every DB request. Defaults to 1s | -| `duration_s` | integer | no | Duration, in seconds, that the core will be utilised. Defaults to 30s | +| `duration_s` | integer | no | Duration, in seconds, that the core will be utilized. Defaults to 30s | | `async` | boolean | no | Set to true to perform the operation in a Sidekiq background worker process | ```bash diff --git a/doc/development/reusing_abstractions.md b/doc/development/reusing_abstractions.md index 59da02ed6fd..fce144f8dc2 100644 --- a/doc/development/reusing_abstractions.md +++ b/doc/development/reusing_abstractions.md @@ -14,13 +14,13 @@ on maintainability, the ability to easily debug problems, or even performance. An example would be to use `ProjectsFinder` in `IssuesFinder` to limit issues to those belonging to a set of projects. While initially this may seem like a good idea, both classes provide a very high level interface with very little control. -This means that `IssuesFinder` may not be able to produce a better optimised +This means that `IssuesFinder` may not be able to produce a better optimized database query, as a large portion of the query is controlled by the internals of `ProjectsFinder`. To work around this problem, you would use the same code used by `ProjectsFinder`, instead of using `ProjectsFinder` itself directly. This allows -you to compose your behaviour better, giving you more control over the behaviour +you to compose your behavior better, giving you more control over the behavior of the code. To illustrate, consider the following code from `IssuableFinder#projects`: @@ -52,7 +52,7 @@ functionality is added to this (high level) interface. Instead of _only_ affecting the cases where this is necessary, it may also start affecting `IssuableFinder` in a negative way. For example, the query produced by `GroupProjectsFinder` may include unnecessary conditions. Since we're using a -finder here, we can't easily opt-out of that behaviour. We could add options to +finder here, we can't easily opt-out of that behavior. We could add options to do so, but then we'd need as many options as we have features. Every option adds two code paths, which means that for four features we have to cover 8 different code paths. @@ -213,6 +213,5 @@ The API provided by Active Record itself, such as the `where` method, `save`, Everything in `app/workers`. -The scheduling of Sidekiq jobs using `SomeWorker.perform_async`, `perform_in`, -etc. Directly invoking a worker using `SomeWorker.new.perform` should be avoided -at all times in application code, though this is fine to use in tests. +Use `SomeWorker.perform_async` or `SomeWorker.perform_in` to schedule Sidekiq +jobs. Never directly invoke a worker using `SomeWorker.new.perform`. -- cgit v1.2.1 From d7eadcc0f30d3bd005f9dfb160dd0a460b3a8f56 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 24 Jul 2019 07:42:45 -0700 Subject: Use a base class for Peek views Introduce a `DetailedView` base class, which is inherited by the Gitaly, Redis, and Rugged views. This reduces code duplication. --- lib/peek/views/detailed_view.rb | 49 ++++++++++++++++++++++++++++++++++++++ lib/peek/views/gitaly.rb | 27 ++++----------------- lib/peek/views/redis_detailed.rb | 23 ++---------------- lib/peek/views/rugged.rb | 35 ++++++++------------------- spec/lib/peek/views/rugged_spec.rb | 3 --- 5 files changed, 65 insertions(+), 72 deletions(-) create mode 100644 lib/peek/views/detailed_view.rb diff --git a/lib/peek/views/detailed_view.rb b/lib/peek/views/detailed_view.rb new file mode 100644 index 00000000000..ebaf46478df --- /dev/null +++ b/lib/peek/views/detailed_view.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Peek + module Views + class DetailedView < View + def results + { + duration: formatted_duration, + calls: calls, + details: details + } + end + + private + + def duration + raise NotImplementedError + end + + def calls + raise NotImplementedError + end + + def call_details + raise NotImplementedError + end + + def format_call_details(call) + raise NotImplementedError + end + + def details + call_details + .sort { |a, b| b[:duration] <=> a[:duration] } + .map(&method(:format_call_details)) + end + + def formatted_duration + ms = duration * 1000 + + if ms >= 1000 + "%.2fms" % ms + else + "%.0fms" % ms + end + end + end + end +end diff --git a/lib/peek/views/gitaly.rb b/lib/peek/views/gitaly.rb index 30f95a10024..067aaf31fbc 100644 --- a/lib/peek/views/gitaly.rb +++ b/lib/peek/views/gitaly.rb @@ -2,7 +2,9 @@ module Peek module Views - class Gitaly < View + class Gitaly < DetailedView + private + def duration ::Gitlab::GitalyClient.query_time end @@ -11,20 +13,8 @@ module Peek ::Gitlab::GitalyClient.get_request_count end - def results - { - duration: formatted_duration, - calls: calls, - details: details - } - end - - private - - def details + def call_details ::Gitlab::GitalyClient.list_call_details - .sort { |a, b| b[:duration] <=> a[:duration] } - .map(&method(:format_call_details)) end def format_call_details(call) @@ -34,15 +24,6 @@ module Peek request: pretty_request || {}) end - def formatted_duration - ms = duration * 1000 - if ms >= 1000 - "%.2fms" % ms - else - "%.0fms" % ms - end - end - def setup_subscribers subscribe 'start_processing.action_controller' do ::Gitlab::GitalyClient.query_time = 0 diff --git a/lib/peek/views/redis_detailed.rb b/lib/peek/views/redis_detailed.rb index 12760c9b75e..c61a1e91282 100644 --- a/lib/peek/views/redis_detailed.rb +++ b/lib/peek/views/redis_detailed.rb @@ -35,36 +35,19 @@ end module Peek module Views - class RedisDetailed < View + class RedisDetailed < DetailedView REDACTED_MARKER = "" def key 'redis' end - def results - { - calls: calls, - duration: formatted_duration, - details: details - } - end - def detail_store ::Gitlab::SafeRequestStore['redis_call_details'] ||= [] end private - def formatted_duration - ms = duration * 1000 - if ms >= 1000 - "%.2fms" % ms - else - "%.0fms" % ms - end - end - def duration detail_store.map { |entry| entry[:duration] }.sum # rubocop:disable CodeReuse/ActiveRecord end @@ -73,10 +56,8 @@ module Peek detail_store.count end - def details + def call_details detail_store - .sort { |a, b| b[:duration] <=> a[:duration] } - .map(&method(:format_call_details)) end def format_call_details(call) diff --git a/lib/peek/views/rugged.rb b/lib/peek/views/rugged.rb index 7b0ab162b15..f0cd520fb8b 100644 --- a/lib/peek/views/rugged.rb +++ b/lib/peek/views/rugged.rb @@ -2,7 +2,15 @@ module Peek module Views - class Rugged < View + class Rugged < DetailedView + def results + return {} unless calls > 0 + + super + end + + private + def duration ::Gitlab::RuggedInstrumentation.query_time end @@ -11,22 +19,8 @@ module Peek ::Gitlab::RuggedInstrumentation.query_count end - def results - return {} unless calls > 0 - - { - duration: formatted_duration, - calls: calls, - details: details - } - end - - private - - def details + def call_details ::Gitlab::RuggedInstrumentation.list_call_details - .sort { |a, b| b[:duration] <=> a[:duration] } - .map(&method(:format_call_details)) end def format_call_details(call) @@ -44,15 +38,6 @@ module Peek end end end - - def formatted_duration - ms = duration * 1000 - if ms >= 1000 - "%.2fms" % ms - else - "%.0fms" % ms - end - end end end end diff --git a/spec/lib/peek/views/rugged_spec.rb b/spec/lib/peek/views/rugged_spec.rb index 715b360953c..8bf996fc6bc 100644 --- a/spec/lib/peek/views/rugged_spec.rb +++ b/spec/lib/peek/views/rugged_spec.rb @@ -27,9 +27,6 @@ describe Peek::Views::Rugged, :request_store do args: [project.repository.raw, 'refs/heads/master'], duration: 0.456) - expect(subject.duration).to be_within(0.00001).of(1.234) - expect(subject.calls).to eq(2) - results = subject.results expect(results[:calls]).to eq(2) expect(results[:duration]).to eq('1234.00ms') -- cgit v1.2.1 From 259493bb4cbce2ab48b7e9b959430888dc9f9d8b Mon Sep 17 00:00:00 2001 From: Andreas Brandl Date: Wed, 24 Jul 2019 17:00:34 +0000 Subject: Enable tablesample count strategy by default https://gitlab.com/gitlab-org/gitlab-ce/issues/58792 --- changelogs/unreleased/ab-count-strategies.yml | 5 +++++ lib/gitlab/database/count.rb | 12 +++++------- lib/gitlab/database/count/exact_count_strategy.rb | 4 ---- lib/gitlab/database/count/reltuples_count_strategy.rb | 4 ---- .../database/count/tablesample_count_strategy.rb | 4 ---- .../gitlab/database/count/exact_count_strategy_spec.rb | 14 -------------- .../database/count/reltuples_count_strategy_spec.rb | 14 -------------- .../database/count/tablesample_count_strategy_spec.rb | 18 ------------------ spec/lib/gitlab/database/count_spec.rb | 15 ++------------- 9 files changed, 12 insertions(+), 78 deletions(-) create mode 100644 changelogs/unreleased/ab-count-strategies.yml diff --git a/changelogs/unreleased/ab-count-strategies.yml b/changelogs/unreleased/ab-count-strategies.yml new file mode 100644 index 00000000000..bd95ff45d6f --- /dev/null +++ b/changelogs/unreleased/ab-count-strategies.yml @@ -0,0 +1,5 @@ +--- +title: Use tablesample approximate counting by default. +merge_request: 31048 +author: +type: performance diff --git a/lib/gitlab/database/count.rb b/lib/gitlab/database/count.rb index f3d37ccd72a..eac61254bdf 100644 --- a/lib/gitlab/database/count.rb +++ b/lib/gitlab/database/count.rb @@ -37,16 +37,14 @@ module Gitlab # @return [Hash] of Model -> count mapping def self.approximate_counts(models, strategies: [TablesampleCountStrategy, ReltuplesCountStrategy, ExactCountStrategy]) strategies.each_with_object({}) do |strategy, counts_by_model| - if strategy.enabled? - models_with_missing_counts = models - counts_by_model.keys + models_with_missing_counts = models - counts_by_model.keys - break counts_by_model if models_with_missing_counts.empty? + break counts_by_model if models_with_missing_counts.empty? - counts = strategy.new(models_with_missing_counts).count + counts = strategy.new(models_with_missing_counts).count - counts.each do |model, count| - counts_by_model[model] = count - end + counts.each do |model, count| + counts_by_model[model] = count end end end diff --git a/lib/gitlab/database/count/exact_count_strategy.rb b/lib/gitlab/database/count/exact_count_strategy.rb index fa6951eda22..0b8fe640bf8 100644 --- a/lib/gitlab/database/count/exact_count_strategy.rb +++ b/lib/gitlab/database/count/exact_count_strategy.rb @@ -23,10 +23,6 @@ module Gitlab rescue *CONNECTION_ERRORS {} end - - def self.enabled? - true - end end end end diff --git a/lib/gitlab/database/count/reltuples_count_strategy.rb b/lib/gitlab/database/count/reltuples_count_strategy.rb index 695f6fa766e..6cd90c01ab2 100644 --- a/lib/gitlab/database/count/reltuples_count_strategy.rb +++ b/lib/gitlab/database/count/reltuples_count_strategy.rb @@ -31,10 +31,6 @@ module Gitlab {} end - def self.enabled? - Gitlab::Database.postgresql? - end - private # Models using single-type inheritance (STI) don't work with diff --git a/lib/gitlab/database/count/tablesample_count_strategy.rb b/lib/gitlab/database/count/tablesample_count_strategy.rb index 7777f31f702..e9387a91a14 100644 --- a/lib/gitlab/database/count/tablesample_count_strategy.rb +++ b/lib/gitlab/database/count/tablesample_count_strategy.rb @@ -28,10 +28,6 @@ module Gitlab {} end - def self.enabled? - Gitlab::Database.postgresql? && Feature.enabled?(:tablesample_counts) - end - private def perform_count(model, estimate) diff --git a/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb index 3991c737a26..0c1be4b4610 100644 --- a/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb +++ b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb @@ -23,18 +23,4 @@ describe Gitlab::Database::Count::ExactCountStrategy do expect(subject).to eq({}) end end - - describe '.enabled?' do - it 'is enabled for PostgreSQL' do - allow(Gitlab::Database).to receive(:postgresql?).and_return(true) - - expect(described_class.enabled?).to be_truthy - end - - it 'is enabled for MySQL' do - allow(Gitlab::Database).to receive(:postgresql?).and_return(false) - - expect(described_class.enabled?).to be_truthy - end - end end diff --git a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb index bd3c66d0548..a528707c9dc 100644 --- a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb +++ b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb @@ -48,18 +48,4 @@ describe Gitlab::Database::Count::ReltuplesCountStrategy do end end end - - describe '.enabled?' do - it 'is enabled for PostgreSQL' do - allow(Gitlab::Database).to receive(:postgresql?).and_return(true) - - expect(described_class.enabled?).to be_truthy - end - - it 'is disabled for MySQL' do - allow(Gitlab::Database).to receive(:postgresql?).and_return(false) - - expect(described_class.enabled?).to be_falsey - end - end end diff --git a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb index 40d810b195b..a57f033b5ed 100644 --- a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb +++ b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb @@ -56,22 +56,4 @@ describe Gitlab::Database::Count::TablesampleCountStrategy do end end end - - describe '.enabled?' do - before do - stub_feature_flags(tablesample_counts: true) - end - - it 'is enabled for PostgreSQL' do - allow(Gitlab::Database).to receive(:postgresql?).and_return(true) - - expect(described_class.enabled?).to be_truthy - end - - it 'is disabled for MySQL' do - allow(Gitlab::Database).to receive(:postgresql?).and_return(false) - - expect(described_class.enabled?).to be_falsey - end - end end diff --git a/spec/lib/gitlab/database/count_spec.rb b/spec/lib/gitlab/database/count_spec.rb index 1d096b8fa7c..71d6633f62f 100644 --- a/spec/lib/gitlab/database/count_spec.rb +++ b/spec/lib/gitlab/database/count_spec.rb @@ -9,24 +9,13 @@ describe Gitlab::Database::Count do let(:models) { [Project, Identity] } context '.approximate_counts' do - context 'selecting strategies' do - let(:strategies) { [double('s1', enabled?: true), double('s2', enabled?: false)] } - - it 'uses only enabled strategies' do - expect(strategies[0]).to receive(:new).and_return(double('strategy1', count: {})) - expect(strategies[1]).not_to receive(:new) - - described_class.approximate_counts(models, strategies: strategies) - end - end - context 'fallbacks' do subject { described_class.approximate_counts(models, strategies: strategies) } let(:strategies) do [ - double('s1', enabled?: true, new: first_strategy), - double('s2', enabled?: true, new: second_strategy) + double('s1', new: first_strategy), + double('s2', new: second_strategy) ] end -- cgit v1.2.1 From 78d57823cabc6ad7844439373a4bed253cfd1f6f Mon Sep 17 00:00:00 2001 From: Ezekiel Kigbo Date: Wed, 24 Jul 2019 17:39:18 +0000 Subject: Removed pluralize function Replaced instance of the `pluralize` js function with `n__` to follow our development guide. --- app/assets/javascripts/commits.js | 5 +++-- .../javascripts/diffs/components/diff_gutter_avatars.vue | 5 +++-- app/assets/javascripts/lib/utils/datetime_utility.js | 15 +++++---------- app/assets/javascripts/lib/utils/text_utility.js | 8 -------- locale/gitlab.pot | 10 ++++++++++ spec/frontend/lib/utils/text_utility_spec.js | 14 -------------- 6 files changed, 21 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/commits.js b/app/assets/javascripts/commits.js index 54e2589c707..7dd75d03ab9 100644 --- a/app/assets/javascripts/commits.js +++ b/app/assets/javascripts/commits.js @@ -1,5 +1,5 @@ import $ from 'jquery'; -import { pluralize } from './lib/utils/text_utility'; +import { n__ } from '~/locale'; import { localTimeAgo } from './lib/utils/datetime_utility'; import Pager from './pager'; import axios from './lib/utils/axios_utils'; @@ -90,9 +90,10 @@ export default class CommitsList { .first() .find('li.commit').length, ); + $commitsHeadersLast .find('span.commits-count') - .text(`${commitsCount} ${pluralize('commit', commitsCount)}`); + .text(n__('%d commit', '%d commits', commitsCount)); } localTimeAgo($processedData.find('.js-timeago')); diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index af5550aec3b..7ede7a4f430 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -1,6 +1,7 @@