From 0cb47d7129c3d5d7bc91d32222ca70d46cb976ca Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 13 Apr 2022 12:10:19 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/assets/javascripts/api.js | 9 + app/assets/javascripts/lib/utils/css_utils.js | 4 + .../pages/profiles/preferences/show/index.js | 2 + .../preferences/components/diffs_colors.vue | 107 ++ .../components/diffs_colors_preview.vue | 231 +++++ .../components/profile_preferences.vue | 15 +- .../profile_preferences_diffs_colors.js | 21 + .../diff_custom_colors_addition.scss | 36 + .../diff_custom_colors_deletion.scss | 36 + .../stylesheets/highlight/themes/dark.scss | 2 + .../stylesheets/highlight/themes/monokai.scss | 5 + .../stylesheets/highlight/themes/none.scss | 5 + .../highlight/themes/solarized-dark.scss | 5 + .../highlight/themes/solarized-light.scss | 5 + .../stylesheets/highlight/themes/white.scss | 5 + .../stylesheets/highlight/white_base.scss | 2 - .../profiles/preferences_controller.rb | 2 + app/helpers/colors_helper.rb | 23 + app/helpers/preferences_helper.rb | 16 + app/models/user.rb | 2 + app/models/user_preference.rb | 3 + .../admin/users/_access_levels.html.haml | 20 +- app/views/layouts/_diffs_colors_css.haml | 20 + app/views/layouts/_startup_css.haml | 3 + app/views/layouts/application.html.haml | 2 +- app/views/profiles/preferences/show.html.haml | 13 + config/application.rb | 2 + ...ulnerability_report_page_size_selector.yml | 8 - .../vulnerability_report_pagination.yml | 2 +- ...01_add_diffs_colors_to_user_preferences.rb | 13 + ..._limit_to_user_preferences_diffs_colors.rb | 15 + ..._index_routes_id_for_project_namespaces.rb | 17 + ...ackfill_namespace_id_for_project_routes.rb | 29 + db/schema_migrations/20220113164801 | 1 + db/schema_migrations/20220113164901 | 1 + db/schema_migrations/20220322023800 | 1 + db/schema_migrations/20220323023800 | 1 + db/structure.sql | 8 +- doc/api/dora/metrics.md | 4 +- doc/development/dangerbot.md | 2 +- doc/user/analytics/ci_cd_analytics.md | 28 - doc/user/analytics/index.md | 115 ++- doc/user/profile/preferences.md | 14 + ...backfill_namespace_id_for_project_route.rb | 58 ++ locale/gitlab.pot | 36 + .../profiles/preferences_controller_spec.rb | 24 + ...er_visits_profile_preferences_page_spec.rb | 8 - spec/frontend/api_spec.js | 32 + .../diffs_colors_preview_spec.js.snap | 915 ++++++++++++++++++ .../components/diffs_colors_preview_spec.js | 23 + .../components/diffs_colors_spec.js | 153 +++ spec/helpers/colors_helper_spec.rb | 89 ++ spec/helpers/preferences_helper_spec.rb | 61 ++ ...ill_namespace_id_for_project_route_spec.rb | 53 + ...ll_namespace_id_for_project_routes_spec.rb | 29 + spec/models/user_preference_spec.rb | 42 + spec/models/user_spec.rb | 6 + workhorse/internal/headers/content_headers.go | 55 +- .../contentprocessor/contentprocessor_test.go | 52 +- 59 files changed, 2342 insertions(+), 149 deletions(-) create mode 100644 app/assets/javascripts/profile/preferences/components/diffs_colors.vue create mode 100644 app/assets/javascripts/profile/preferences/components/diffs_colors_preview.vue create mode 100644 app/assets/javascripts/profile/preferences/profile_preferences_diffs_colors.js create mode 100644 app/assets/stylesheets/highlight/diff_custom_colors_addition.scss create mode 100644 app/assets/stylesheets/highlight/diff_custom_colors_deletion.scss create mode 100644 app/helpers/colors_helper.rb create mode 100644 app/views/layouts/_diffs_colors_css.haml delete mode 100644 config/feature_flags/development/vulnerability_report_page_size_selector.yml create mode 100644 db/migrate/20220113164801_add_diffs_colors_to_user_preferences.rb create mode 100644 db/migrate/20220113164901_add_text_limit_to_user_preferences_diffs_colors.rb create mode 100644 db/post_migrate/20220322023800_add_tmp_index_routes_id_for_project_namespaces.rb create mode 100644 db/post_migrate/20220323023800_backfill_namespace_id_for_project_routes.rb create mode 100644 db/schema_migrations/20220113164801 create mode 100644 db/schema_migrations/20220113164901 create mode 100644 db/schema_migrations/20220322023800 create mode 100644 db/schema_migrations/20220323023800 create mode 100644 lib/gitlab/background_migration/backfill_namespace_id_for_project_route.rb create mode 100644 spec/frontend/profile/preferences/components/__snapshots__/diffs_colors_preview_spec.js.snap create mode 100644 spec/frontend/profile/preferences/components/diffs_colors_preview_spec.js create mode 100644 spec/frontend/profile/preferences/components/diffs_colors_spec.js create mode 100644 spec/helpers/colors_helper_spec.rb create mode 100644 spec/lib/gitlab/background_migration/backfill_namespace_id_for_project_route_spec.rb create mode 100644 spec/migrations/backfill_namespace_id_for_project_routes_spec.rb diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index d2856d99ef0..186a0ab01e7 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -42,6 +42,7 @@ const Api = { projectMergeRequestVersionsPath: '/api/:version/projects/:id/merge_requests/:mrid/versions', projectRunnersPath: '/api/:version/projects/:id/runners', projectProtectedBranchesPath: '/api/:version/projects/:id/protected_branches', + projectProtectedBranchesNamePath: '/api/:version/projects/:id/protected_branches/:name', projectSearchPath: '/api/:version/projects/:id/search', projectSharePath: '/api/:version/projects/:id/share', projectMilestonesPath: '/api/:version/projects/:id/milestones', @@ -372,6 +373,14 @@ const Api = { .then(({ data }) => data); }, + projectProtectedBranch(id, branchName) { + const url = Api.buildUrl(Api.projectProtectedBranchesNamePath) + .replace(':id', encodeURIComponent(id)) + .replace(':name', branchName); + + return axios.get(url).then(({ data }) => data); + }, + projectSearch(id, options = {}) { const url = Api.buildUrl(Api.projectSearchPath).replace(':id', encodeURIComponent(id)); diff --git a/app/assets/javascripts/lib/utils/css_utils.js b/app/assets/javascripts/lib/utils/css_utils.js index 76ac442a470..e4f68dd1b6c 100644 --- a/app/assets/javascripts/lib/utils/css_utils.js +++ b/app/assets/javascripts/lib/utils/css_utils.js @@ -19,3 +19,7 @@ export function loadCSSFile(path) { } }); } + +export function getCssVariable(variable) { + return getComputedStyle(document.documentElement).getPropertyValue(variable).trim(); +} diff --git a/app/assets/javascripts/pages/profiles/preferences/show/index.js b/app/assets/javascripts/pages/profiles/preferences/show/index.js index 2922ff88721..76939434680 100644 --- a/app/assets/javascripts/pages/profiles/preferences/show/index.js +++ b/app/assets/javascripts/pages/profiles/preferences/show/index.js @@ -1,3 +1,5 @@ import initProfilePreferences from '~/profile/preferences/profile_preferences_bundle'; +import initProfilePreferencesDiffsColors from '~/profile/preferences/profile_preferences_diffs_colors'; initProfilePreferences(); +initProfilePreferencesDiffsColors(); diff --git a/app/assets/javascripts/profile/preferences/components/diffs_colors.vue b/app/assets/javascripts/profile/preferences/components/diffs_colors.vue new file mode 100644 index 00000000000..1992819ab82 --- /dev/null +++ b/app/assets/javascripts/profile/preferences/components/diffs_colors.vue @@ -0,0 +1,107 @@ + + diff --git a/app/assets/javascripts/profile/preferences/components/diffs_colors_preview.vue b/app/assets/javascripts/profile/preferences/components/diffs_colors_preview.vue new file mode 100644 index 00000000000..74dd2d5628a --- /dev/null +++ b/app/assets/javascripts/profile/preferences/components/diffs_colors_preview.vue @@ -0,0 +1,231 @@ + + diff --git a/app/assets/javascripts/profile/preferences/components/profile_preferences.vue b/app/assets/javascripts/profile/preferences/components/profile_preferences.vue index 757a66ef148..7542f81a361 100644 --- a/app/assets/javascripts/profile/preferences/components/profile_preferences.vue +++ b/app/assets/javascripts/profile/preferences/components/profile_preferences.vue @@ -45,7 +45,7 @@ export default { return { isSubmitEnabled: true, darkModeOnCreate: null, - darkModeOnSubmit: null, + schemeOnCreate: null, }; }, computed: { @@ -61,6 +61,7 @@ export default { this.formEl.addEventListener('ajax:success', this.handleSuccess); this.formEl.addEventListener('ajax:error', this.handleError); this.darkModeOnCreate = this.darkModeSelected(); + this.schemeOnCreate = this.getSelectedScheme(); }, beforeDestroy() { this.formEl.removeEventListener('ajax:beforeSend', this.handleLoading); @@ -76,15 +77,19 @@ export default { const themeId = new FormData(this.formEl).get('user[theme_id]'); return this.applicationThemes[themeId] ?? null; }, + getSelectedScheme() { + return new FormData(this.formEl).get('user[color_scheme_id]'); + }, handleLoading() { this.isSubmitEnabled = false; - this.darkModeOnSubmit = this.darkModeSelected(); }, handleSuccess(customEvent) { // Reload the page if the theme has changed from light to dark mode or vice versa - // to correctly load all required styles. - const modeChanged = this.darkModeOnCreate ? !this.darkModeOnSubmit : this.darkModeOnSubmit; - if (modeChanged) { + // or if color scheme has changed to correctly load all required styles. + if ( + this.darkModeOnCreate !== this.darkModeSelected() || + this.schemeOnCreate !== this.getSelectedScheme() + ) { window.location.reload(); return; } diff --git a/app/assets/javascripts/profile/preferences/profile_preferences_diffs_colors.js b/app/assets/javascripts/profile/preferences/profile_preferences_diffs_colors.js new file mode 100644 index 00000000000..1b200187610 --- /dev/null +++ b/app/assets/javascripts/profile/preferences/profile_preferences_diffs_colors.js @@ -0,0 +1,21 @@ +import Vue from 'vue'; +import DiffsColors from './components/diffs_colors.vue'; + +export default () => { + const el = document.querySelector('#js-profile-preferences-diffs-colors-app'); + + if (!el) return false; + + const { deletion, addition } = el.dataset; + + return new Vue({ + el, + provide: { + deletion, + addition, + }, + render(createElement) { + return createElement(DiffsColors); + }, + }); +}; diff --git a/app/assets/stylesheets/highlight/diff_custom_colors_addition.scss b/app/assets/stylesheets/highlight/diff_custom_colors_addition.scss new file mode 100644 index 00000000000..30895a55711 --- /dev/null +++ b/app/assets/stylesheets/highlight/diff_custom_colors_addition.scss @@ -0,0 +1,36 @@ +/** +* CSS variables used below are declared in `app/views/layouts/_diffs_colors_css.haml` +*/ +.diff-custom-addition-color { + .code { + .line_holder { + .diff-line-num, + .line-coverage, + .line-codequality, + .line_content { + &.new { + &:not(.hll) { + background: var(--diff-addition-color); + } + + &.line_content span.idiff { + background: var(--diff-addition-color) !important; + } + + &::before, + a { + mix-blend-mode: luminosity; + } + } + } + } + + .gd { + background-color: var(--diff-addition-color); + } + } + + .idiff.addition { + background: var(--diff-addition-color) !important; + } +} diff --git a/app/assets/stylesheets/highlight/diff_custom_colors_deletion.scss b/app/assets/stylesheets/highlight/diff_custom_colors_deletion.scss new file mode 100644 index 00000000000..a8ab43909eb --- /dev/null +++ b/app/assets/stylesheets/highlight/diff_custom_colors_deletion.scss @@ -0,0 +1,36 @@ +/** +* CSS variables used below are declared in `app/views/layouts/_diffs_colors_css.haml` +*/ +.diff-custom-deletion-color { + .code { + .line_holder { + .diff-line-num, + .line-coverage, + .line-codequality, + .line_content { + &.old { + &:not(.hll) { + background: var(--diff-deletion-color); + } + + &.line_content span.idiff { + background: var(--diff-deletion-color) !important; + } + + &::before, + a { + mix-blend-mode: luminosity; + } + } + } + } + + .gd { + background-color: var(--diff-deletion-color); + } + } + + .idiff.deletion { + background: var(--diff-deletion-color) !important; + } +} diff --git a/app/assets/stylesheets/highlight/themes/dark.scss b/app/assets/stylesheets/highlight/themes/dark.scss index 09dcedff43c..c51b1f04757 100644 --- a/app/assets/stylesheets/highlight/themes/dark.scss +++ b/app/assets/stylesheets/highlight/themes/dark.scss @@ -120,6 +120,8 @@ $dark-il: #de935f; --color-hljs-selector-id: #{$dark-nn}; --color-hljs-selector-attr: #{$dark-nt}; --color-hljs-selector-pseudo: #{$dark-nd}; + --default-diff-color-deletion: #ff3333; + --default-diff-color-addition: #288f2a; } .code.dark { diff --git a/app/assets/stylesheets/highlight/themes/monokai.scss b/app/assets/stylesheets/highlight/themes/monokai.scss index 6faf1cffdef..226bb44f0e7 100644 --- a/app/assets/stylesheets/highlight/themes/monokai.scss +++ b/app/assets/stylesheets/highlight/themes/monokai.scss @@ -89,6 +89,11 @@ $monokai-gd: #f92672; $monokai-gi: #a6e22e; $monokai-gh: #75715e; +:root { + --default-diff-color-deletion: #c87872; + --default-diff-color-addition: #678528; +} + .code.monokai { // Line numbers .file-line-num { diff --git a/app/assets/stylesheets/highlight/themes/none.scss b/app/assets/stylesheets/highlight/themes/none.scss index 9c28d9463dc..7a36aba8be7 100644 --- a/app/assets/stylesheets/highlight/themes/none.scss +++ b/app/assets/stylesheets/highlight/themes/none.scss @@ -9,6 +9,11 @@ background-color: $white-normal; } +:root { + --default-diff-color-deletion: #b4b4b4; + --default-diff-color-addition: #b4b4b4; +} + .code.none { // Line numbers .file-line-num { diff --git a/app/assets/stylesheets/highlight/themes/solarized-dark.scss b/app/assets/stylesheets/highlight/themes/solarized-dark.scss index c9f889c79fc..acd401e1694 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-dark.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-dark.scss @@ -92,6 +92,11 @@ $solarized-dark-vg: #268bd2; $solarized-dark-vi: #268bd2; $solarized-dark-il: #2aa198; +:root { + --default-diff-color-deletion: #ff362c; + --default-diff-color-addition: #647e0e; +} + .code.solarized-dark { // Line numbers .file-line-num { diff --git a/app/assets/stylesheets/highlight/themes/solarized-light.scss b/app/assets/stylesheets/highlight/themes/solarized-light.scss index 0108d7e496f..ddcecc4cbcf 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-light.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-light.scss @@ -94,6 +94,11 @@ $solarized-light-vg: #268bd2; $solarized-light-vi: #268bd2; $solarized-light-il: #2aa198; +:root { + --default-diff-color-deletion: #dc322f; + --default-diff-color-addition: #859900; +} + @mixin match-line { color: $black-transparent; background: $solarized-light-matchline-bg; diff --git a/app/assets/stylesheets/highlight/themes/white.scss b/app/assets/stylesheets/highlight/themes/white.scss index ed1d9c924c0..8698e448c94 100644 --- a/app/assets/stylesheets/highlight/themes/white.scss +++ b/app/assets/stylesheets/highlight/themes/white.scss @@ -3,3 +3,8 @@ @include conflict-colors('white'); } + +:root { + --default-diff-color-deletion: #eb919b; + --default-diff-color-addition: #a0f5b4; +} \ No newline at end of file diff --git a/app/assets/stylesheets/highlight/white_base.scss b/app/assets/stylesheets/highlight/white_base.scss index 91d8f4a1ba5..20a36d2e8b1 100644 --- a/app/assets/stylesheets/highlight/white_base.scss +++ b/app/assets/stylesheets/highlight/white_base.scss @@ -149,7 +149,6 @@ pre.code, .diff-line-num { &.old { background-color: $line-number-old; - border-color: $line-removed-dark; a { color: scale-color($line-number-old, $red: -30%, $green: -30%, $blue: -30%); @@ -158,7 +157,6 @@ pre.code, &.new { background-color: $line-number-new; - border-color: $line-added-dark; a { color: scale-color($line-number-new, $red: -30%, $green: -30%, $blue: -30%); diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb index adecb56ea38..820b6520f6c 100644 --- a/app/controllers/profiles/preferences_controller.rb +++ b/app/controllers/profiles/preferences_controller.rb @@ -36,6 +36,8 @@ class Profiles::PreferencesController < Profiles::ApplicationController def preferences_param_names [ :color_scheme_id, + :diffs_deletion_color, + :diffs_addition_color, :layout, :dashboard, :project_view, diff --git a/app/helpers/colors_helper.rb b/app/helpers/colors_helper.rb new file mode 100644 index 00000000000..bc72122220a --- /dev/null +++ b/app/helpers/colors_helper.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module ColorsHelper + HEX_COLOR_PATTERN = /\A\#(?:[0-9A-Fa-f]{3}){1,2}\Z/.freeze + + def hex_color_to_rgb_array(hex_color) + raise ArgumentError, "invalid hex color `#{hex_color}`" unless hex_color =~ HEX_COLOR_PATTERN + + hex_color.length == 7 ? hex_color[1, 7].scan(/.{2}/).map(&:hex) : hex_color[1, 4].scan(/./).map { |v| (v * 2).hex } + end + + def rgb_array_to_hex_color(rgb_array) + raise ArgumentError, "invalid RGB array `#{rgb_array}`" unless rgb_array_valid?(rgb_array) + + "##{rgb_array.map{ "%02x" % _1 }.join}" + end + + private + + def rgb_array_valid?(rgb_array) + rgb_array.is_a?(Array) && rgb_array.length == 3 && rgb_array.all?{ _1 >= 0 && _1 <= 255 } + end +end diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index 6a8c39b5b15..39a57e786ed 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -82,6 +82,22 @@ module PreferencesHelper Gitlab::TabWidth.css_class_for_user(current_user) end + def user_diffs_colors + { + deletion: current_user&.diffs_deletion_color.presence, + addition: current_user&.diffs_addition_color.presence + }.compact + end + + def custom_diff_color_classes + return if request.path == profile_preferences_path + + classes = [] + classes << 'diff-custom-addition-color' if current_user&.diffs_addition_color.presence + classes << 'diff-custom-deletion-color' if current_user&.diffs_deletion_color.presence + classes + end + def language_choices options_for_select( selectable_locales_with_translation_level.sort, diff --git a/app/models/user.rb b/app/models/user.rb index 027ea70958c..743ba4d229c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -324,6 +324,8 @@ class User < ApplicationRecord :setup_for_company, :setup_for_company=, :render_whitespace_in_code, :render_whitespace_in_code=, :markdown_surround_selection, :markdown_surround_selection=, + :diffs_deletion_color, :diffs_deletion_color=, + :diffs_addition_color, :diffs_addition_color=, to: :user_preference delegate :path, to: :namespace, allow_nil: true, prefix: true diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb index 7687430cfd1..9b4c0a2527a 100644 --- a/app/models/user_preference.rb +++ b/app/models/user_preference.rb @@ -19,6 +19,9 @@ class UserPreference < ApplicationRecord greater_than_or_equal_to: Gitlab::TabWidth::MIN, less_than_or_equal_to: Gitlab::TabWidth::MAX } + validates :diffs_deletion_color, :diffs_addition_color, + format: { with: ColorsHelper::HEX_COLOR_PATTERN }, + allow_blank: true ignore_columns :experience_level, remove_with: '14.10', remove_after: '2021-03-22' diff --git a/app/views/admin/users/_access_levels.html.haml b/app/views/admin/users/_access_levels.html.haml index 1e4c3f3bb62..51e6af56377 100644 --- a/app/views/admin/users/_access_levels.html.haml +++ b/app/views/admin/users/_access_levels.html.haml @@ -11,7 +11,7 @@ .col-sm-2.col-form-label.gl-pt-0 = f.label :can_create_group .col-sm-10 - = f.check_box :can_create_group + = f.gitlab_ui_checkbox_component :can_create_group, '' .form-group.row .col-sm-2.col-form-label.gl-pt-0 @@ -39,10 +39,7 @@ = f.label :external .hidden{ data: user_internal_regex_data } .col-sm-10.gl-display-flex.gl-align-items-baseline - = f.check_box :external do - = s_('AdminUsers|External') - %p.light.gl-pl-2 - = s_('AdminUsers|External users cannot see internal or private projects unless access is explicitly granted. Also, external users cannot create projects, groups, or personal snippets.') + = f.gitlab_ui_checkbox_component :external, s_('AdminUsers|External users cannot see internal or private projects unless access is explicitly granted. Also, external users cannot create projects, groups, or personal snippets.') %row.hidden#warning_external_automatically_set = gl_badge_tag s_('AdminUsers|Automatically marked as default internal user'), variant: :warning @@ -50,12 +47,9 @@ - @user.credit_card_validation || @user.build_credit_card_validation = f.fields_for :credit_card_validation do |ff| .col-sm-2.col-form-label.gl-pt-0 - = ff.label s_("AdminUsers|Validate user account") + = ff.label s_('AdminUsers|Validate user account') .col-sm-10.gl-display-flex.gl-align-items-baseline - = ff.check_box :credit_card_validated_at, checked: @user.credit_card_validated_at.present? - .gl-pl-2 - .light - = s_('AdminUsers|User is validated and can use free CI minutes on shared runners.') - .gl-text-gray-600 - = s_('AdminUsers|A user can validate themselves by inputting a credit/debit card, or an admin can manually validate a user.') - + = ff.gitlab_ui_checkbox_component :credit_card_validated_at, + s_('AdminUsers|User is validated and can use free CI minutes on shared runners.'), + help_text: s_('AdminUsers|A user can validate themselves by inputting a credit/debit card, or an admin can manually validate a user.'), + checkbox_options: { checked: @user.credit_card_validated_at.present? } diff --git a/app/views/layouts/_diffs_colors_css.haml b/app/views/layouts/_diffs_colors_css.haml new file mode 100644 index 00000000000..d2efa392bd9 --- /dev/null +++ b/app/views/layouts/_diffs_colors_css.haml @@ -0,0 +1,20 @@ +- deletion_color = local_assigns.fetch(:deletion, nil) +- addition_color = local_assigns.fetch(:addition, nil) + +- if deletion_color.present? || request.path == profile_preferences_path + = stylesheet_link_tag_defer "highlight/diff_custom_colors_deletion" +- if deletion_color.present? + - deletion_color_rgb = hex_color_to_rgb_array(deletion_color).join(',') + :css + :root { + --diff-deletion-color: rgba(#{deletion_color_rgb},0.2); + } + +- if addition_color.present? || request.path == profile_preferences_path + = stylesheet_link_tag_defer "highlight/diff_custom_colors_addition" +- if addition_color.present? + - addition_color_rgb = hex_color_to_rgb_array(addition_color).join(',') + :css + :root { + --diff-addition-color: rgba(#{addition_color_rgb},0.2); + } diff --git a/app/views/layouts/_startup_css.haml b/app/views/layouts/_startup_css.haml index 67c871b95f5..64a86cf319e 100644 --- a/app/views/layouts/_startup_css.haml +++ b/app/views/layouts/_startup_css.haml @@ -1,6 +1,9 @@ - startup_filename_default = user_application_theme == 'gl-dark' ? 'dark' : 'general' - startup_filename = local_assigns.fetch(:startup_filename, nil) || startup_filename_default +- diffs_colors = user_diffs_colors %style = Rails.application.assets_manifest.find_sources("themes/#{user_application_theme_css_filename}.css").first.to_s.html_safe if user_application_theme_css_filename = Rails.application.assets_manifest.find_sources("startup/startup-#{startup_filename}.css").first.to_s.html_safe + += render 'layouts/diffs_colors_css', diffs_colors if diffs_colors.present? || request.path == profile_preferences_path diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 26e3d9b3b92..bdab5d7ea07 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -1,6 +1,6 @@ - page_classes = page_class << @html_class - page_classes = page_classes.flatten.compact -- body_classes = [user_application_theme, user_tab_width, @body_class, client_class_list] +- body_classes = [user_application_theme, user_tab_width, @body_class, client_class_list, *custom_diff_color_classes] !!! 5 %html{ lang: I18n.locale, class: page_classes } diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 48be2001c9c..ddd3b4c2d1c 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -44,6 +44,19 @@ .col-sm-12 %hr + .row.js-preferences-form.js-search-settings-section + .col-lg-4.profile-settings-sidebar#diffs-colors + %h4.gl-mt-0 + = s_('Preferences|Diff colors') + %p + = s_('Preferences|Customize the colors of removed and added lines in diffs.') + .col-lg-8 + .form-group + #js-profile-preferences-diffs-colors-app{ data: user_diffs_colors } + + .col-sm-12 + %hr + .row.js-preferences-form.js-search-settings-section .col-lg-4.profile-settings-sidebar#behavior %h4.gl-mt-0 diff --git a/config/application.rb b/config/application.rb index b07af18d9c9..0216c3e7b9f 100644 --- a/config/application.rb +++ b/config/application.rb @@ -314,6 +314,8 @@ module Gitlab config.assets.precompile << "themes/*.css" config.assets.precompile << "highlight/themes/*.css" + config.assets.precompile << "highlight/diff_custom_colors_addition.css" + config.assets.precompile << "highlight/diff_custom_colors_deletion.css" # Import gitlab-svgs directly from vendored directory config.assets.paths << "#{config.root}/node_modules/@gitlab/svgs/dist" diff --git a/config/feature_flags/development/vulnerability_report_page_size_selector.yml b/config/feature_flags/development/vulnerability_report_page_size_selector.yml deleted file mode 100644 index b410ee27adb..00000000000 --- a/config/feature_flags/development/vulnerability_report_page_size_selector.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: vulnerability_report_page_size_selector -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82438 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356888 -milestone: '14.10' -type: development -group: group::threat insights -default_enabled: false diff --git a/config/feature_flags/development/vulnerability_report_pagination.yml b/config/feature_flags/development/vulnerability_report_pagination.yml index 71639f6790b..677cc1efe11 100644 --- a/config/feature_flags/development/vulnerability_report_pagination.yml +++ b/config/feature_flags/development/vulnerability_report_pagination.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351975 milestone: '14.8' type: development group: group::threat insights -default_enabled: false +default_enabled: true diff --git a/db/migrate/20220113164801_add_diffs_colors_to_user_preferences.rb b/db/migrate/20220113164801_add_diffs_colors_to_user_preferences.rb new file mode 100644 index 00000000000..00e8e574722 --- /dev/null +++ b/db/migrate/20220113164801_add_diffs_colors_to_user_preferences.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddDiffsColorsToUserPreferences < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + # rubocop:disable Migration/AddLimitToTextColumns + # limit is added in 20220113164901_add_text_limit_to_user_preferences_diffs_colors.rb + def change + add_column :user_preferences, :diffs_deletion_color, :text + add_column :user_preferences, :diffs_addition_color, :text + end + # rubocop:enable Migration/AddLimitToTextColumns +end diff --git a/db/migrate/20220113164901_add_text_limit_to_user_preferences_diffs_colors.rb b/db/migrate/20220113164901_add_text_limit_to_user_preferences_diffs_colors.rb new file mode 100644 index 00000000000..9ae1c875194 --- /dev/null +++ b/db/migrate/20220113164901_add_text_limit_to_user_preferences_diffs_colors.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddTextLimitToUserPreferencesDiffsColors < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_text_limit :user_preferences, :diffs_deletion_color, 7 + add_text_limit :user_preferences, :diffs_addition_color, 7 + end + + def down + remove_text_limit :user_preferences, :diffs_addition_color + remove_text_limit :user_preferences, :diffs_deletion_color + end +end diff --git a/db/post_migrate/20220322023800_add_tmp_index_routes_id_for_project_namespaces.rb b/db/post_migrate/20220322023800_add_tmp_index_routes_id_for_project_namespaces.rb new file mode 100644 index 00000000000..bc775514b75 --- /dev/null +++ b/db/post_migrate/20220322023800_add_tmp_index_routes_id_for_project_namespaces.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddTmpIndexRoutesIdForProjectNamespaces < Gitlab::Database::Migration[1.0] + INDEX_NAME = 'tmp_index_for_project_namespace_id_migration_on_routes' + + disable_ddl_transaction! + + def up + # Temporary index to be removed in 15.0 + # https://gitlab.com/gitlab-org/gitlab/-/issues/352353 + add_concurrent_index :routes, :id, where: "namespace_id IS NULL AND source_type = 'Project'", name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :routes, INDEX_NAME + end +end diff --git a/db/post_migrate/20220323023800_backfill_namespace_id_for_project_routes.rb b/db/post_migrate/20220323023800_backfill_namespace_id_for_project_routes.rb new file mode 100644 index 00000000000..b938688be2c --- /dev/null +++ b/db/post_migrate/20220323023800_backfill_namespace_id_for_project_routes.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class BackfillNamespaceIdForProjectRoutes < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + MIGRATION = 'BackfillNamespaceIdForProjectRoute' + INTERVAL = 2.minutes + BATCH_SIZE = 1_000 + MAX_BATCH_SIZE = 10_000 + SUB_BATCH_SIZE = 200 + + def up + queue_batched_background_migration( + MIGRATION, + :routes, + :id, + job_interval: INTERVAL, + batch_size: BATCH_SIZE, + max_batch_size: MAX_BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + Gitlab::Database::BackgroundMigration::BatchedMigration + .for_configuration(MIGRATION, :routes, :id, []) + .delete_all + end +end diff --git a/db/schema_migrations/20220113164801 b/db/schema_migrations/20220113164801 new file mode 100644 index 00000000000..8354489ac31 --- /dev/null +++ b/db/schema_migrations/20220113164801 @@ -0,0 +1 @@ +71526ea198c64d23a35f06804f30068591e937df22d74c262fdec9ecf04bf7d4 \ No newline at end of file diff --git a/db/schema_migrations/20220113164901 b/db/schema_migrations/20220113164901 new file mode 100644 index 00000000000..977ec8bb51b --- /dev/null +++ b/db/schema_migrations/20220113164901 @@ -0,0 +1 @@ +b157cec5eab77665ae57f02647c39dc0fb167d78e1894b395c46f59d791ab3e0 \ No newline at end of file diff --git a/db/schema_migrations/20220322023800 b/db/schema_migrations/20220322023800 new file mode 100644 index 00000000000..99e5cbfa7c3 --- /dev/null +++ b/db/schema_migrations/20220322023800 @@ -0,0 +1 @@ +1aefb5950063a060de1ec20b0808a5488b238b36d86120c34ac5a128c212984e \ No newline at end of file diff --git a/db/schema_migrations/20220323023800 b/db/schema_migrations/20220323023800 new file mode 100644 index 00000000000..ff735118ede --- /dev/null +++ b/db/schema_migrations/20220323023800 @@ -0,0 +1 @@ +1681c19d1f41a05c3dfeded70d128989afb4a81a2e04aacc8879c2c1ab766733 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a3d18a997da..55bafa0750d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -21335,7 +21335,11 @@ CREATE TABLE user_preferences ( experience_level smallint, view_diffs_file_by_file boolean DEFAULT false NOT NULL, gitpod_enabled boolean DEFAULT false NOT NULL, - markdown_surround_selection boolean DEFAULT true NOT NULL + markdown_surround_selection boolean DEFAULT true NOT NULL, + diffs_deletion_color text, + diffs_addition_color text, + CONSTRAINT check_89bf269f41 CHECK ((char_length(diffs_deletion_color) <= 7)), + CONSTRAINT check_d07ccd35f7 CHECK ((char_length(diffs_addition_color) <= 7)) ); CREATE SEQUENCE user_preferences_id_seq @@ -29663,6 +29667,8 @@ CREATE INDEX tmp_index_for_namespace_id_migration_on_routes ON routes USING btre CREATE INDEX tmp_index_for_null_project_namespace_id ON projects USING btree (id) WHERE (project_namespace_id IS NULL); +CREATE INDEX tmp_index_for_project_namespace_id_migration_on_routes ON routes USING btree (id) WHERE ((namespace_id IS NULL) AND ((source_type)::text = 'Project'::text)); + CREATE INDEX tmp_index_issues_on_issue_type_and_id ON issues USING btree (issue_type, id); CREATE INDEX tmp_index_members_on_state ON members USING btree (state) WHERE (state = 2); diff --git a/doc/api/dora/metrics.md b/doc/api/dora/metrics.md index 75c7372b534..f5373d02156 100644 --- a/doc/api/dora/metrics.md +++ b/doc/api/dora/metrics.md @@ -24,7 +24,7 @@ GET /projects/:id/dora/metrics | Attribute | Type | Required | Description | |-------------- |-------- |----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `id` | integer/string | yes | The ID or [URL-encoded path of the project](../index.md#namespaced-path-encoding) can be accessed by the authenticated user. | -| `metric` | string | yes | The [metric name](../../user/analytics/ci_cd_analytics.md#supported-metrics-in-gitlab). One of `deployment_frequency`, `lead_time_for_changes` or `time_to_restore_service`.| +| `metric` | string | yes | The metric name: `deployment_frequency`, `lead_time_for_changes` or `time_to_restore_service`.| | `start_date` | string | no | Date range to start from. ISO 8601 Date format, for example `2021-03-01`. Default is 3 months ago. | | `end_date` | string | no | Date range to end at. ISO 8601 Date format, for example `2021-03-01`. Default is the current date. | | `interval` | string | no | The bucketing interval. One of `all`, `monthly` or `daily`. Default is `daily`. | @@ -64,7 +64,7 @@ GET /groups/:id/dora/metrics | Attribute | Type | Required | Description | |-------------- |-------- |----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `id` | integer/string | yes | The ID or [URL-encoded path of the project](../index.md#namespaced-path-encoding) can be accessed by the authenticated user. | -| `metric` | string | yes | The [metric name](../../user/analytics/ci_cd_analytics.md#supported-metrics-in-gitlab). One of `deployment_frequency`, `lead_time_for_changes`, `time_to_restore_service` or `change_failure_rate`. | +| `metric` | string | yes | One of `deployment_frequency`, `lead_time_for_changes`, `time_to_restore_service` or `change_failure_rate`. | | `start_date` | string | no | Date range to start from. ISO 8601 Date format, for example `2021-03-01`. Default is 3 months ago. | | `end_date` | string | no | Date range to end at. ISO 8601 Date format, for example `2021-03-01`. Default is the current date. | | `interval` | string | no | The bucketing interval. One of `all`, `monthly` or `daily`. Default is `daily`. | diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md index 9bf0fbe1d78..0f83a41676a 100644 --- a/doc/development/dangerbot.md +++ b/doc/development/dangerbot.md @@ -142,7 +142,7 @@ To enable the Dangerfile on another existing GitLab project, complete the follow 1. Create a `Dangerfile` with the following content: ```ruby - require_relative "lib/gitlab-dangerfiles" + require "gitlab-dangerfiles" Gitlab::Dangerfiles.for_project(self, &:import_defaults) ``` diff --git a/doc/user/analytics/ci_cd_analytics.md b/doc/user/analytics/ci_cd_analytics.md index 8e231d18f41..f0de1e58891 100644 --- a/doc/user/analytics/ci_cd_analytics.md +++ b/doc/user/analytics/ci_cd_analytics.md @@ -34,34 +34,6 @@ To view CI/CD analytics: 1. On the top bar, select **Menu > Projects** and find your project. 1. On the left sidebar, select **Analytics > CI/CD Analytics**. -## DevOps Research and Assessment (DORA) key metrics **(ULTIMATE)** - -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/275991) in GitLab 13.7. -> - [Added support](https://gitlab.com/gitlab-org/gitlab/-/issues/291746) for lead time for changes in GitLab 13.10. - -The DevOps Research and Assessment ([DORA](https://cloud.google.com/blog/products/devops-sre/the-2019-accelerate-state-of-devops-elite-performance-productivity-and-scaling)) -team developed several key metrics that you can use as performance indicators for software development -teams: - -- Deployment frequency: How often an organization successfully releases to production. -- Lead time for changes: The amount of time it takes for code to reach production. -- Change failure rate: The percentage of deployments that cause a failure in production. -- Time to restore service: How long it takes for an organization to recover from a failure in - production. - -### Supported metrics in GitLab - -The following table shows the supported metrics, at which level they are supported, and which GitLab version (API and UI) they were introduced: - -| Metric | Level | API version | Chart (UI) version | Comments | -|---------------------------|---------------------|--------------------------------------|---------------------------------------|-----------| -| `deployment_frequency` | Project-level | [13.7+](../../api/dora/metrics.md) | [13.8+](#view-deployment-frequency-chart) | The [old API endpoint](../../api/dora4_project_analytics.md) was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/323713) in 13.10. | -| `deployment_frequency` | Group-level | [13.10+](../../api/dora/metrics.md) | [13.12+](#view-deployment-frequency-chart) | | -| `lead_time_for_changes` | Project-level | [13.10+](../../api/dora/metrics.md) | [13.11+](#view-lead-time-for-changes-chart) | Unit in seconds. Aggregation method is median. | -| `lead_time_for_changes` | Group-level | [13.10+](../../api/dora/metrics.md) | [14.0+](#view-lead-time-for-changes-chart) | Unit in seconds. Aggregation method is median. | -| `change_failure_rate` | Project/Group-level | To be supported | To be supported | | -| `time_to_restore_service` | Project/Group-level | To be supported | To be supported | | - ## View deployment frequency chart **(ULTIMATE)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/275991) in GitLab 13.8. diff --git a/doc/user/analytics/index.md b/doc/user/analytics/index.md index 01ee9857060..d3879c26c60 100644 --- a/doc/user/analytics/index.md +++ b/doc/user/analytics/index.md @@ -54,52 +54,82 @@ The following analytics features are available for users to create personalized Be sure to review the documentation page for this feature for GitLab tier requirements. +## DevOps Research and Assessment (DORA) key metrics **(ULTIMATE)** + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/275991) in GitLab 13.7. +> - [Added support](https://gitlab.com/gitlab-org/gitlab/-/issues/291746) for lead time for changes in GitLab 13.10. + +The [DevOps Research and Assessment (DORA)](https://cloud.google.com/blog/products/devops-sre/using-the-four-keys-to-measure-your-devops-performance) +team developed several key metrics that you can use as performance indicators for software development +teams. + +### Deployment frequency + +Deployment frequency is the frequency of successful deployments to production (hourly, daily, weekly, monthly, or yearly). +This measures how often you deliver value to end users. A higher deployment frequency means you can +get feedback sooner and iterate faster to deliver improvements and features. GitLab measures this as the number of +deployments to a production environment in the given time period. + +Deployment frequency displays in several charts: + +- [Group-level value stream analytics](../group/value_stream_analytics/index.md) +- [Project-level value stream analytics](value_stream_analytics.md) +- [CI/CD analytics](ci_cd_analytics.md) + +### Lead time for changes + +Lead time for changes measures the time to deliver a feature once it has been developed, +as described in [Measuring DevOps Performance](https://devops.com/measuring-devops-performance/). + +Lead time for changes displays in several charts: + +- [Group-level value stream analytics](../group/value_stream_analytics/index.md) +- [Project-level value stream analytics](value_stream_analytics.md) +- [CI/CD analytics](ci_cd_analytics.md) + +### Time to restore service + +Time to restore service measures how long it takes an organization to recover from a failure in production. +GitLab measures this as the average time required to close the incidents +in the given time period. This assumes: + +- All incidents are related to a production environment. +- Incidents and deployments have a strictly one-to-one relationship. An incident is related to only +one production deployment, and any production deployment is related to no more than one incident). + +To retrieve metrics for time to restore service, use the [GraphQL](../../api/graphql/reference/index.md) or the [REST](../../api/dora/metrics.md) APIs. + +### Change failure rate + +Change failure rate measures the percentage of deployments that cause a failure in production. GitLab measures this as the number +of incidents divided by the number of deployments to a +production environment in the given time period. This assumes: + +- All incidents are related to a production environment. +- Incidents and deployments have a strictly one-to-one relationship. An incident is related to only +one production deployment, and any production deployment is related to no +more than one incident. + +NOTE: +GitLab does not support the change failure rate metric. + +### Supported DORA metrics in GitLab + +| Metric | Level | API | UI chart | Comments | +|---------------------------|-------------------------|-------------------------------------|---------------------------------------|-------------------------------| +| `deployment_frequency` | Project | [GitLab 13.7 and later](../../api/dora/metrics.md) | GitLab 14.8 and later | The [previous API endpoint](../../api/dora4_project_analytics.md) was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/323713) in 13.10. | +| `deployment_frequency` | Group | [GitLab 13.10 and later](../../api/dora/metrics.md) | GitLab 13.12 and later | | +| `lead_time_for_changes` | Project | [GitLab 13.10 and later](../../api/dora/metrics.md) | GitLab 13.11 and later | Unit in seconds. Aggregation method is median. | +| `lead_time_for_changes` | Group | [GitLab 13.10 and later](../../api/dora/metrics.md) | GitLab 14.0 and later | Unit in seconds. Aggregation method is median. | +| `time_to_restore_service` | Project and group | [GitLab 14.9 and later](../../api/dora/metrics.md) | Not supported | | +| `change_failure_rate` | Project and group | Not supported | Not supported | | + ## Definitions We use the following terms to describe GitLab analytics: - **Cycle time:** The duration of only the execution work. Cycle time is often displayed in combination with the lead time, which is longer than the cycle time. GitLab measures cycle time from the earliest commit of a [linked issue's merge request](../project/issues/crosslinking_issues.md) to when that issue is closed. The cycle time approach underestimates the lead time because merge request creation is always later than commit time. GitLab displays cycle time in [group-level Value Stream Analytics](../group/value_stream_analytics/index.md) and [project-level Value Stream Analytics](../analytics/value_stream_analytics.md). - **Deploys:** The total number of successful deployments to production in the given time frame (across all applicable projects). GitLab displays deploys in [group-level Value Stream Analytics](../group/value_stream_analytics/index.md) and [project-level Value Stream Analytics](value_stream_analytics.md). -- **DORA (DevOps Research and Assessment)** ["Four Keys"](https://cloud.google.com/blog/products/devops-sre/using-the-four-keys-to-measure-your-devops-performance): - - **Speed/Velocity** - - - **Deployment frequency:** The relative frequency of successful deployments to production - (hourly, daily, weekly, monthly, or yearly). - This measures how often you are delivering value to end users. A higher deployment - frequency means you are able to get feedback and iterate faster to deliver - improvements and features. GitLab measures this as the number of deployments to a - [production environment](../../ci/environments/index.md#deployment-tier-of-environments) in - the given time period. - GitLab displays deployment frequency in [group-level Value Stream Analytics](../group/value_stream_analytics/index.md) and [project-level Value Stream Analytics](value_stream_analytics.md). - - **Lead Time for Changes:** The time it takes for a commit to get into production. GitLab - measures this as the median duration between merge request merge and deployment to a - [production environment](../../ci/environments/index.md#deployment-tier-of-environments) for - all MRs deployed in the given time period. This measure under estimates lead time because - merge time is always later than commit time. The - [standard definition](https://github.com/GoogleCloudPlatform/fourkeys/blob/main/METRICS.md#lead-time-for-changes) uses median commit time. - [An issue exists](https://gitlab.com/gitlab-org/gitlab/-/issues/328459) to start - measuring from "issue first commit" as a better proxy, although still imperfect. - - - **Stability** - - **Change Failure Rate:** The percentage of deployments causing a failure in production. - GitLab measures this as the number of [incidents](../../operations/incident_management/incidents.md) - divided by the number of deployments to a - [production environment](../../ci/environments/index.md#deployment-tier-of-environments) in - the given time period. This assumes: - - - All incidents are related to a production environment. - - Incidents and deployments have a strictly one-to-one relationship (meaning any incident is - related to only one production deployment, and any production deployment is related to no - more than one incident). - - - **Time to Restore Service:** How long it takes an organization to recover from a failure in - production. GitLab measures this as the average time required to close the - [incidents](../../operations/incident_management/incidents.md) in the given time period. - This assumes: - - - All incidents are related to a [production environment](../../ci/environments/index.md#deployment-tier-of-environments). - - Incidents and deployments have a strictly one-to-one relationship (meaning any incident is related to only one production deployment, and any production deployment is related to no more than one incident). - - **Lead time:** The duration of your value stream, from start to finish. Different to [Lead time for changes](#lead-time-for-changes). Often displayed in combination with "cycle time," which is shorter. GitLab measures lead time from issue creation to issue close. GitLab displays lead @@ -121,8 +151,3 @@ with "plan" and ends with "monitor". GitLab helps you track your value stream us - **Velocity:** The total issue burden completed in some period of time. The burden is usually measured in points or weight, often per sprint. For example, your velocity may be "30 points per sprint". GitLab measures velocity as the total points or weight of issues closed in a given period of time. - -## Lead time for changes - -"Lead Time for Changes" differs from "Lead Time" because it "focuses on measuring only the time to -deliver a feature once it has been developed", as described in ([Measuring DevOps Performance](https://devops.com/measuring-devops-performance/)). diff --git a/doc/user/profile/preferences.md b/doc/user/profile/preferences.md index 48160bb97ac..ecd6e83efa1 100644 --- a/doc/user/profile/preferences.md +++ b/doc/user/profile/preferences.md @@ -89,6 +89,20 @@ The default syntax theme is White, and you can choose among 5 different themes: Introduced in GitLab 13.6, the themes [Solarized](https://gitlab.com/gitlab-org/gitlab/-/issues/221034) and [Monokai](https://gitlab.com/gitlab-org/gitlab/-/issues/221034) also apply to the [Web IDE](../project/web_ide/index.md) and [Snippets](../snippets.md). +## Diff colors + +A diff compares the old/removed content with the new/added content (e.g. when +[reviewing a merge request](../project/merge_requests/reviews/index.md#review-a-merge-request) or in a +[Markdown inline diff](../markdown.md#inline-diff)). +Typically, the colors red and green are used for removed and added lines in diffs. +The exact colors depend on the selected [syntax highlighting theme](#syntax-highlighting-theme). +The colors may lead to difficulties in case of red–green color blindness. + +For this reason, you can customize the following colors: + +- Color for removed lines +- Color for added lines + ## Behavior The following settings allow you to customize the behavior of the GitLab layout diff --git a/lib/gitlab/background_migration/backfill_namespace_id_for_project_route.rb b/lib/gitlab/background_migration/backfill_namespace_id_for_project_route.rb new file mode 100644 index 00000000000..1f0d606f001 --- /dev/null +++ b/lib/gitlab/background_migration/backfill_namespace_id_for_project_route.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Backfills the `routes.namespace_id` column, by setting it to project.project_namespace_id + class BackfillNamespaceIdForProjectRoute + include Gitlab::Database::DynamicModelHelpers + + def perform(start_id, end_id, batch_table, batch_column, sub_batch_size, pause_ms) + parent_batch_relation = relation_scoped_to_range(batch_table, batch_column, start_id, end_id) + + parent_batch_relation.each_batch(column: batch_column, of: sub_batch_size) do |sub_batch| + cleanup_gin_index('routes') + + batch_metrics.time_operation(:update_all) do + ActiveRecord::Base.connection.execute <<~SQL + WITH route_and_ns(route_id, project_namespace_id) AS #{::Gitlab::Database::AsWithMaterialized.materialized_if_supported} ( + #{sub_batch.to_sql} + ) + UPDATE routes + SET namespace_id = route_and_ns.project_namespace_id + FROM route_and_ns + WHERE id = route_and_ns.route_id + SQL + end + + pause_ms = [0, pause_ms].max + sleep(pause_ms * 0.001) + end + end + + def batch_metrics + @batch_metrics ||= Gitlab::Database::BackgroundMigration::BatchMetrics.new + end + + private + + def cleanup_gin_index(table_name) + sql = "select indexname::text from pg_indexes where tablename = '#{table_name}' and indexdef ilike '%gin%'" + index_names = ActiveRecord::Base.connection.select_values(sql) + + index_names.each do |index_name| + ActiveRecord::Base.connection.execute("select gin_clean_pending_list('#{index_name}')") + end + end + + def relation_scoped_to_range(source_table, source_key_column, start_id, stop_id) + define_batchable_model(source_table, connection: ActiveRecord::Base.connection) + .joins('INNER JOIN projects ON routes.source_id = projects.id') + .where(source_key_column => start_id..stop_id) + .where(namespace_id: nil) + .where(source_type: 'Project') + .where.not(projects: { project_namespace_id: nil }) + .select("routes.id, projects.project_namespace_id") + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6c388b966fc..0fe099cd84a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -28363,6 +28363,12 @@ msgstr "" msgid "Preferences|Choose what content you want to see on your homepage." msgstr "" +msgid "Preferences|Color for added lines" +msgstr "" + +msgid "Preferences|Color for removed lines" +msgstr "" + msgid "Preferences|Configure how dates and times display for you." msgstr "" @@ -28372,6 +28378,12 @@ msgstr "" msgid "Preferences|Customize the appearance of the application header and navigation sidebar." msgstr "" +msgid "Preferences|Customize the colors of removed and added lines in diffs." +msgstr "" + +msgid "Preferences|Diff colors" +msgstr "" + msgid "Preferences|Display time in 24-hour format" msgstr "" @@ -28408,6 +28420,9 @@ msgstr "" msgid "Preferences|Navigation theme" msgstr "" +msgid "Preferences|Preview" +msgstr "" + msgid "Preferences|Project overview content" msgstr "" @@ -33404,6 +33419,9 @@ msgstr "" msgid "SecurityConfiguration|Vulnerability details and statistics in the merge request" msgstr "" +msgid "SecurityOrchestration| and " +msgstr "" + msgid "SecurityOrchestration| or " msgstr "" @@ -33632,6 +33650,9 @@ msgstr "" msgid "SecurityOrchestration|Summary" msgstr "" +msgid "SecurityOrchestration|The following branches do not exist on this development project: %{branches}. Please review all branches to ensure the values are accurate before updating this policy." +msgstr "" + msgid "SecurityOrchestration|There was a problem creating the new security policy" msgstr "" @@ -36507,6 +36528,12 @@ msgstr "" msgid "SuggestedColors|Crimson" msgstr "" +msgid "SuggestedColors|Current addition color" +msgstr "" + +msgid "SuggestedColors|Current removal color" +msgstr "" + msgid "SuggestedColors|Dark coral" msgstr "" @@ -36522,6 +36549,12 @@ msgstr "" msgid "SuggestedColors|Deep violet" msgstr "" +msgid "SuggestedColors|Default addition color" +msgstr "" + +msgid "SuggestedColors|Default removal color" +msgstr "" + msgid "SuggestedColors|Gray" msgstr "" @@ -36540,6 +36573,9 @@ msgstr "" msgid "SuggestedColors|Medium sea green" msgstr "" +msgid "SuggestedColors|Orange" +msgstr "" + msgid "SuggestedColors|Red" msgstr "" diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb index b7870a63f9d..7add3a72337 100644 --- a/spec/controllers/profiles/preferences_controller_spec.rb +++ b/spec/controllers/profiles/preferences_controller_spec.rb @@ -46,6 +46,8 @@ RSpec.describe Profiles::PreferencesController do it "changes the user's preferences" do prefs = { color_scheme_id: '1', + diffs_deletion_color: '#123456', + diffs_addition_color: '#abcdef', dashboard: 'stars', theme_id: '2', first_day_of_week: '1', @@ -84,5 +86,27 @@ RSpec.describe Profiles::PreferencesController do expect(response.parsed_body['type']).to eq('alert') end end + + context 'on invalid diffs colors setting' do + it 'responds with error for diffs_deletion_color' do + prefs = { diffs_deletion_color: '#1234567' } + + go params: prefs + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.parsed_body['message']).to eq _('Failed to save preferences.') + expect(response.parsed_body['type']).to eq('alert') + end + + it 'responds with error for diffs_addition_color' do + prefs = { diffs_addition_color: '#1234567' } + + go params: prefs + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.parsed_body['message']).to eq _('Failed to save preferences.') + expect(response.parsed_body['type']).to eq('alert') + end + end end end diff --git a/spec/features/profiles/user_visits_profile_preferences_page_spec.rb b/spec/features/profiles/user_visits_profile_preferences_page_spec.rb index e19e29bf63a..4c61e8d45e4 100644 --- a/spec/features/profiles/user_visits_profile_preferences_page_spec.rb +++ b/spec/features/profiles/user_visits_profile_preferences_page_spec.rb @@ -18,14 +18,6 @@ RSpec.describe 'User visits the profile preferences page', :js do end describe 'User changes their syntax highlighting theme', :js do - it 'creates a flash message' do - choose 'user_color_scheme_id_5' - - wait_for_requests - - expect_preferences_saved_message - end - it 'updates their preference' do choose 'user_color_scheme_id_5' diff --git a/spec/frontend/api_spec.js b/spec/frontend/api_spec.js index 9f273ce5ba3..c303d470c7b 100644 --- a/spec/frontend/api_spec.js +++ b/spec/frontend/api_spec.js @@ -1733,4 +1733,36 @@ describe('Api', () => { }); }); }); + + describe('projectProtectedBranch', () => { + const branchName = 'new-branch-name'; + const dummyProjectId = 5; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${dummyProjectId}/protected_branches/${branchName}`; + + it('returns 404 for non-existing branch', () => { + jest.spyOn(axios, 'get'); + + mock.onGet(expectedUrl).replyOnce(httpStatus.NOT_FOUND, { + message: '404 Not found', + }); + + return Api.projectProtectedBranch(dummyProjectId, branchName).catch((error) => { + expect(error.response.status).toBe(httpStatus.NOT_FOUND); + expect(axios.get).toHaveBeenCalledWith(expectedUrl); + }); + }); + + it('returns 200 with branch information', () => { + const expectedObj = { name: branchName }; + + jest.spyOn(axios, 'get'); + + mock.onGet(expectedUrl).replyOnce(httpStatus.OK, expectedObj); + + return Api.projectProtectedBranch(dummyProjectId, branchName).then((data) => { + expect(data).toEqual(expectedObj); + expect(axios.get).toHaveBeenCalledWith(expectedUrl); + }); + }); + }); }); diff --git a/spec/frontend/profile/preferences/components/__snapshots__/diffs_colors_preview_spec.js.snap b/spec/frontend/profile/preferences/components/__snapshots__/diffs_colors_preview_spec.js.snap new file mode 100644 index 00000000000..3025a2f87ae --- /dev/null +++ b/spec/frontend/profile/preferences/components/__snapshots__/diffs_colors_preview_spec.js.snap @@ -0,0 +1,915 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`DiffsColorsPreview component renders diff colors preview 1`] = ` +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + # + + Removed + + content + + + + + + + + # + + Added + + content + + +
+ + + + + v + + + + = + + + + 1 + + + + + + + + v + + + + = + + + + 1 + + +
+ + + + + s + + + + = + + + + "string" + + + + + + + + s + + + + = + + + + "string" + + +
+ + + + + + + +
+ + + + + for + + + + i + + + + in + + + + range + + + ( + + + - + + + 10 + + + , + + + + 10 + + + ): + + + + + + + + for + + + + i + + + + in + + + + range + + + ( + + + - + + + 10 + + + , + + + + 10 + + + ): + + +
+ + + + + + + + + print + + + ( + + + i + + + + + + + + + 1 + + + ) + + + + + + + + + + + + print + + + ( + + + i + + + + + + + + + 1 + + + ) + + +
+ + + + + + + +
+ + + + + class + + + + LinkedList + + + ( + + + object + + + ): + + + + + + + + class + + + + LinkedList + + + ( + + + object + + + ): + + +
+ + + + + + + + + def + + + + __init__ + + + ( + + + self + + + , + + + + x + + + ): + + + + + + + + + + + + def + + + + __init__ + + + ( + + + self + + + , + + + + x + + + ): + + +
+ + + + + + + + + self + + + . + + + val + + + + = + + + + x + + + + + + + + + + + + self + + + . + + + val + + + + = + + + + x + + +
+ + + + + + + + + self + + + . + + + next + + + + = + + + + None + + + + + + + + + + + + self + + + . + + + next + + + + = + + + + None + + +
+
+`; diff --git a/spec/frontend/profile/preferences/components/diffs_colors_preview_spec.js b/spec/frontend/profile/preferences/components/diffs_colors_preview_spec.js new file mode 100644 index 00000000000..e60602ab336 --- /dev/null +++ b/spec/frontend/profile/preferences/components/diffs_colors_preview_spec.js @@ -0,0 +1,23 @@ +import { shallowMount } from '@vue/test-utils'; +import DiffsColorsPreview from '~/profile/preferences/components/diffs_colors_preview.vue'; + +describe('DiffsColorsPreview component', () => { + let wrapper; + + function createComponent() { + wrapper = shallowMount(DiffsColorsPreview); + } + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + it('renders diff colors preview', () => { + expect(wrapper.element).toMatchSnapshot(); + }); +}); diff --git a/spec/frontend/profile/preferences/components/diffs_colors_spec.js b/spec/frontend/profile/preferences/components/diffs_colors_spec.js new file mode 100644 index 00000000000..02f501a0b06 --- /dev/null +++ b/spec/frontend/profile/preferences/components/diffs_colors_spec.js @@ -0,0 +1,153 @@ +import { shallowMount } from '@vue/test-utils'; +import { s__ } from '~/locale'; +import ColorPicker from '~/vue_shared/components/color_picker/color_picker.vue'; +import DiffsColors from '~/profile/preferences/components/diffs_colors.vue'; +import DiffsColorsPreview from '~/profile/preferences/components/diffs_colors_preview.vue'; +import * as CssUtils from '~/lib/utils/css_utils'; + +describe('DiffsColors component', () => { + let wrapper; + + const defaultInjectedProps = { + addition: '#00ff00', + deletion: '#ff0000', + }; + + const initialSuggestedColors = { + '#d99530': s__('SuggestedColors|Orange'), + '#1f75cb': s__('SuggestedColors|Blue'), + }; + + const findColorPickers = () => wrapper.findAllComponents(ColorPicker); + + function createComponent(provide = {}) { + wrapper = shallowMount(DiffsColors, { + provide: { + ...defaultInjectedProps, + ...provide, + }, + }); + } + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + it('mounts', () => { + createComponent(); + + expect(wrapper.exists()).toBe(true); + }); + + describe('preview', () => { + it('should render preview', () => { + createComponent(); + + expect(wrapper.findComponent(DiffsColorsPreview).exists()).toBe(true); + }); + + it('should set preview classes', () => { + createComponent(); + + expect(wrapper.attributes('class')).toBe( + 'diff-custom-addition-color diff-custom-deletion-color', + ); + }); + + it.each([ + [{ addition: null }, 'diff-custom-deletion-color'], + [{ deletion: null }, 'diff-custom-addition-color'], + ])('should not set preview class if color not set', (provide, expectedClass) => { + createComponent(provide); + + expect(wrapper.attributes('class')).toBe(expectedClass); + }); + + it.each([ + [{}, '--diff-deletion-color: rgba(255,0,0,0.2); --diff-addition-color: rgba(0,255,0,0.2);'], + [{ addition: null }, '--diff-deletion-color: rgba(255,0,0,0.2);'], + [{ deletion: null }, '--diff-addition-color: rgba(0,255,0,0.2);'], + ])('should set correct CSS variables', (provide, expectedStyle) => { + createComponent(provide); + + expect(wrapper.attributes('style')).toBe(expectedStyle); + }); + }); + + describe('color pickers', () => { + it('should render both color pickers', () => { + createComponent(); + + const colorPickers = findColorPickers(); + + expect(colorPickers.length).toBe(2); + expect(colorPickers.at(0).props()).toMatchObject({ + label: s__('Preferences|Color for removed lines'), + value: '#ff0000', + state: true, + }); + expect(colorPickers.at(1).props()).toMatchObject({ + label: s__('Preferences|Color for added lines'), + value: '#00ff00', + state: true, + }); + }); + + describe('suggested colors', () => { + const suggestedColors = () => findColorPickers().at(0).props('suggestedColors'); + + it('contains initial suggested colors', () => { + createComponent(); + + expect(suggestedColors()).toMatchObject(initialSuggestedColors); + }); + + it('contains default diff colors of theme', () => { + jest.spyOn(CssUtils, 'getCssVariable').mockImplementation((variable) => { + if (variable === '--default-diff-color-addition') return '#111111'; + if (variable === '--default-diff-color-deletion') return '#222222'; + return '#000000'; + }); + + createComponent(); + + expect(suggestedColors()).toMatchObject({ + '#111111': s__('SuggestedColors|Default addition color'), + '#222222': s__('SuggestedColors|Default removal color'), + }); + }); + + it('contains current diff colors if set', () => { + createComponent(); + + expect(suggestedColors()).toMatchObject({ + [defaultInjectedProps.addition]: s__('SuggestedColors|Current addition color'), + [defaultInjectedProps.deletion]: s__('SuggestedColors|Current removal color'), + }); + }); + + it.each([ + [ + { addition: null }, + s__('SuggestedColors|Current removal color'), + s__('SuggestedColors|Current addition color'), + ], + [ + { deletion: null }, + s__('SuggestedColors|Current addition color'), + s__('SuggestedColors|Current removal color'), + ], + ])( + 'does not contain current diff color if not set %p', + (provide, expectedToContain, expectNotToContain) => { + createComponent(provide); + + const suggestedColorsLabels = Object.values(suggestedColors()); + expect(suggestedColorsLabels).toContain(expectedToContain); + expect(suggestedColorsLabels).not.toContain(expectNotToContain); + }, + ); + }); + }); +}); diff --git a/spec/helpers/colors_helper_spec.rb b/spec/helpers/colors_helper_spec.rb new file mode 100644 index 00000000000..ca5cafb7ebe --- /dev/null +++ b/spec/helpers/colors_helper_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ColorsHelper do + using RSpec::Parameterized::TableSyntax + + describe '#hex_color_to_rgb_array' do + context 'valid hex color' do + where(:hex_color, :rgb_array) do + '#000000' | [0, 0, 0] + '#aaaaaa' | [170, 170, 170] + '#cCcCcC' | [204, 204, 204] + '#FFFFFF' | [255, 255, 255] + '#000abc' | [0, 10, 188] + '#123456' | [18, 52, 86] + '#a1b2c3' | [161, 178, 195] + '#000' | [0, 0, 0] + '#abc' | [170, 187, 204] + '#321' | [51, 34, 17] + '#7E2' | [119, 238, 34] + '#fFf' | [255, 255, 255] + end + + with_them do + it 'returns correct RGB array' do + expect(helper.hex_color_to_rgb_array(hex_color)).to eq(rgb_array) + end + end + end + + context 'invalid hex color' do + where(:hex_color) { ['', '0', '#00', '#ffff', '#1234567', 'invalid', [], 1, nil] } + + with_them do + it 'raise ArgumentError' do + expect { helper.hex_color_to_rgb_array(hex_color) }.to raise_error(ArgumentError) + end + end + end + end + + describe '#rgb_array_to_hex_color' do + context 'valid RGB array' do + where(:rgb_array, :hex_color) do + [0, 0, 0] | '#000000' + [0, 0, 255] | '#0000ff' + [0, 255, 0] | '#00ff00' + [255, 0, 0] | '#ff0000' + [12, 34, 56] | '#0c2238' + [222, 111, 88] | '#de6f58' + [255, 255, 255] | '#ffffff' + end + + with_them do + it 'returns correct hex color' do + expect(helper.rgb_array_to_hex_color(rgb_array)).to eq(hex_color) + end + end + end + + context 'invalid RGB array' do + where(:rgb_array) do + [ + '', + '#000000', + 0, + nil, + [], + [0], + [0, 0], + [0, 0, 0, 0], + [-1, 0, 0], + [0, -1, 0], + [0, 0, -1], + [256, 0, 0], + [0, 256, 0], + [0, 0, 256] + ] + end + + with_them do + it 'raise ArgumentError' do + expect { helper.rgb_array_to_hex_color(rgb_array) }.to raise_error(ArgumentError) + end + end + end + end +end diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index 8c13afc2b45..01235c7bb51 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -145,6 +145,67 @@ RSpec.describe PreferencesHelper do end end + describe '#user_diffs_colors' do + context 'with a user' do + it "returns user's diffs colors" do + stub_user(diffs_addition_color: '#123456', diffs_deletion_color: '#abcdef') + + expect(helper.user_diffs_colors).to eq({ addition: '#123456', deletion: '#abcdef' }) + end + + it 'omits property if nil' do + stub_user(diffs_addition_color: '#123456', diffs_deletion_color: nil) + + expect(helper.user_diffs_colors).to eq({ addition: '#123456' }) + end + + it 'omits property if blank' do + stub_user(diffs_addition_color: '', diffs_deletion_color: '#abcdef') + + expect(helper.user_diffs_colors).to eq({ deletion: '#abcdef' }) + end + end + + context 'without a user' do + it 'returns no properties' do + stub_user + + expect(helper.user_diffs_colors).to eq({}) + end + end + end + + describe '#custom_diff_color_classes' do + context 'with a user' do + it 'returns color classes' do + stub_user(diffs_addition_color: '#123456', diffs_deletion_color: '#abcdef') + + expect(helper.custom_diff_color_classes) + .to match_array(%w[diff-custom-addition-color diff-custom-deletion-color]) + end + + it 'omits property if nil' do + stub_user(diffs_addition_color: '#123456', diffs_deletion_color: nil) + + expect(helper.custom_diff_color_classes).to match_array(['diff-custom-addition-color']) + end + + it 'omits property if blank' do + stub_user(diffs_addition_color: '', diffs_deletion_color: '#abcdef') + + expect(helper.custom_diff_color_classes).to match_array(['diff-custom-deletion-color']) + end + end + + context 'without a user' do + it 'returns no classes' do + stub_user + + expect(helper.custom_diff_color_classes).to match_array([]) + end + end + end + describe '#language_choices' do include StubLanguagesTranslationPercentage diff --git a/spec/lib/gitlab/background_migration/backfill_namespace_id_for_project_route_spec.rb b/spec/lib/gitlab/background_migration/backfill_namespace_id_for_project_route_spec.rb new file mode 100644 index 00000000000..2dcd4645c84 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_namespace_id_for_project_route_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillNamespaceIdForProjectRoute do + let(:migration) { described_class.new } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:routes) { table(:routes) } + + let(:namespace1) { namespaces.create!(name: 'batchtest1', type: 'Group', path: 'space1') } + let(:namespace2) { namespaces.create!(name: 'batchtest2', type: 'Group', parent_id: namespace1.id, path: 'space2') } + let(:namespace3) { namespaces.create!(name: 'batchtest3', type: 'Group', parent_id: namespace2.id, path: 'space3') } + + let(:proj_namespace1) { namespaces.create!(name: 'proj1', path: 'proj1', type: 'Project', parent_id: namespace1.id) } + let(:proj_namespace2) { namespaces.create!(name: 'proj2', path: 'proj2', type: 'Project', parent_id: namespace2.id) } + let(:proj_namespace3) { namespaces.create!(name: 'proj3', path: 'proj3', type: 'Project', parent_id: namespace3.id) } + let(:proj_namespace4) { namespaces.create!(name: 'proj4', path: 'proj4', type: 'Project', parent_id: namespace3.id) } + + # rubocop:disable Layout/LineLength + let(:proj1) { projects.create!(name: 'proj1', path: 'proj1', namespace_id: namespace1.id, project_namespace_id: proj_namespace1.id) } + let(:proj2) { projects.create!(name: 'proj2', path: 'proj2', namespace_id: namespace2.id, project_namespace_id: proj_namespace2.id) } + let(:proj3) { projects.create!(name: 'proj3', path: 'proj3', namespace_id: namespace3.id, project_namespace_id: proj_namespace3.id) } + let(:proj4) { projects.create!(name: 'proj4', path: 'proj4', namespace_id: namespace3.id, project_namespace_id: proj_namespace4.id) } + # rubocop:enable Layout/LineLength + + let!(:namespace_route1) { routes.create!(path: 'space1', source_id: namespace1.id, source_type: 'Namespace') } + let!(:namespace_route2) { routes.create!(path: 'space1/space2', source_id: namespace2.id, source_type: 'Namespace') } + let!(:namespace_route3) { routes.create!(path: 'space1/space3', source_id: namespace3.id, source_type: 'Namespace') } + + let!(:proj_route1) { routes.create!(path: 'space1/proj1', source_id: proj1.id, source_type: 'Project') } + let!(:proj_route2) { routes.create!(path: 'space1/space2/proj2', source_id: proj2.id, source_type: 'Project') } + let!(:proj_route3) { routes.create!(path: 'space1/space3/proj3', source_id: proj3.id, source_type: 'Project') } + let!(:proj_route4) { routes.create!(path: 'space1/space3/proj4', source_id: proj4.id, source_type: 'Project') } + + subject(:perform_migration) { migration.perform(proj_route1.id, proj_route4.id, :routes, :id, 2, 0) } + + it 'backfills namespace_id for the selected records', :aggregate_failures do + perform_migration + + expected_namespaces = [proj_namespace1.id, proj_namespace2.id, proj_namespace3.id, proj_namespace4.id] + + expected_projects = [proj_route1.id, proj_route2.id, proj_route3.id, proj_route4.id] + expect(routes.where.not(namespace_id: nil).pluck(:id)).to match_array(expected_projects) + expect(routes.where.not(namespace_id: nil).pluck(:namespace_id)).to match_array(expected_namespaces) + end + + it 'tracks timings of queries' do + expect(migration.batch_metrics.timings).to be_empty + + expect { perform_migration }.to change { migration.batch_metrics.timings } + end +end diff --git a/spec/migrations/backfill_namespace_id_for_project_routes_spec.rb b/spec/migrations/backfill_namespace_id_for_project_routes_spec.rb new file mode 100644 index 00000000000..28edd17731f --- /dev/null +++ b/spec/migrations/backfill_namespace_id_for_project_routes_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe BackfillNamespaceIdForProjectRoutes, :migration do + let(:migration) { described_class::MIGRATION } + + describe '#up' do + it 'schedules background jobs for each batch of group members' do + migrate! + + expect(migration).to have_scheduled_batched_migration( + table_name: :routes, + column_name: :id, + interval: described_class::INTERVAL + ) + end + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end +end diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index d4491aacd9f..2492521c634 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -5,6 +5,48 @@ require 'spec_helper' RSpec.describe UserPreference do let(:user_preference) { create(:user_preference) } + describe 'validations' do + describe 'diffs_deletion_color and diffs_addition_color' do + using RSpec::Parameterized::TableSyntax + + where(color: [ + '#000000', + '#123456', + '#abcdef', + '#AbCdEf', + '#ffffff', + '#fFfFfF', + '#000', + '#123', + '#abc', + '#AbC', + '#fff', + '#fFf', + '' + ]) + + with_them do + it { is_expected.to allow_value(color).for(:diffs_deletion_color) } + it { is_expected.to allow_value(color).for(:diffs_addition_color) } + end + + where(color: [ + '#1', + '#12', + '#1234', + '#12345', + '#1234567', + '123456', + '#12345x' + ]) + + with_them do + it { is_expected.not_to allow_value(color).for(:diffs_deletion_color) } + it { is_expected.not_to allow_value(color).for(:diffs_addition_color) } + end + end + end + describe 'notes filters global keys' do it 'contains expected values' do expect(UserPreference::NOTES_FILTERS.keys).to match_array([:all_notes, :only_comments, :only_activity]) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d8267c7db99..05e38efea80 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -69,6 +69,12 @@ RSpec.describe User do it { is_expected.to delegate_method(:markdown_surround_selection).to(:user_preference) } it { is_expected.to delegate_method(:markdown_surround_selection=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:diffs_deletion_color).to(:user_preference) } + it { is_expected.to delegate_method(:diffs_deletion_color=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:diffs_addition_color).to(:user_preference) } + it { is_expected.to delegate_method(:diffs_addition_color=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:job_title).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:job_title=).to(:user_detail).with_arguments(:args).allow_nil } diff --git a/workhorse/internal/headers/content_headers.go b/workhorse/internal/headers/content_headers.go index 9c33ddb8c8a..8cca3d97e82 100644 --- a/workhorse/internal/headers/content_headers.go +++ b/workhorse/internal/headers/content_headers.go @@ -8,28 +8,37 @@ import ( ) var ( - ImageTypeRegex = regexp.MustCompile(`^image/*`) - SvgMimeTypeRegex = regexp.MustCompile(`^image/svg\+xml$`) + javaScriptTypeRegex = regexp.MustCompile(`^(text|application)\/javascript$`) - TextTypeRegex = regexp.MustCompile(`^text/*`) + imageTypeRegex = regexp.MustCompile(`^image/*`) + svgMimeTypeRegex = regexp.MustCompile(`^image/svg\+xml$`) - VideoTypeRegex = regexp.MustCompile(`^video/*`) + textTypeRegex = regexp.MustCompile(`^text/*`) - PdfTypeRegex = regexp.MustCompile(`application\/pdf`) + videoTypeRegex = regexp.MustCompile(`^video/*`) - AttachmentRegex = regexp.MustCompile(`^attachment`) - InlineRegex = regexp.MustCompile(`^inline`) + pdfTypeRegex = regexp.MustCompile(`application\/pdf`) + + attachmentRegex = regexp.MustCompile(`^attachment`) + inlineRegex = regexp.MustCompile(`^inline`) ) // Mime types that can't be inlined. Usually subtypes of main types -var forbiddenInlineTypes = []*regexp.Regexp{SvgMimeTypeRegex} +var forbiddenInlineTypes = []*regexp.Regexp{svgMimeTypeRegex} // Mime types that can be inlined. We can add global types like "image/" or // specific types like "text/plain". If there is a specific type inside a global // allowed type that can't be inlined we must add it to the forbiddenInlineTypes var. // One example of this is the mime type "image". We allow all images to be // inlined except for SVGs. -var allowedInlineTypes = []*regexp.Regexp{ImageTypeRegex, TextTypeRegex, VideoTypeRegex, PdfTypeRegex} +var allowedInlineTypes = []*regexp.Regexp{imageTypeRegex, textTypeRegex, videoTypeRegex, pdfTypeRegex} + +const ( + svgContentType = "image/svg+xml" + textPlainContentType = "text/plain; charset=utf-8" + attachmentDispositionText = "attachment" + inlineDispositionText = "inline" +) func SafeContentHeaders(data []byte, contentDisposition string) (string, string) { contentType := safeContentType(data) @@ -40,16 +49,24 @@ func SafeContentHeaders(data []byte, contentDisposition string) (string, string) func safeContentType(data []byte) string { // Special case for svg because DetectContentType detects it as text if svg.Is(data) { - return "image/svg+xml" + return svgContentType } // Override any existing Content-Type header from other ResponseWriters contentType := http.DetectContentType(data) + // http.DetectContentType does not support JavaScript and would only + // return text/plain. But for cautionary measures, just in case they start supporting + // it down the road and start returning application/javascript, we want to handle it now + // to avoid regressions. + if isType(contentType, javaScriptTypeRegex) { + return textPlainContentType + } + // If the content is text type, we set to plain, because we don't // want to render it inline if they're html or javascript - if isType(contentType, TextTypeRegex) { - return "text/plain; charset=utf-8" + if isType(contentType, textTypeRegex) { + return textPlainContentType } return contentType @@ -58,7 +75,7 @@ func safeContentType(data []byte) string { func safeContentDisposition(contentType string, contentDisposition string) string { // If the existing disposition is attachment we return that. This allow us // to force a download from GitLab (ie: RawController) - if AttachmentRegex.MatchString(contentDisposition) { + if attachmentRegex.MatchString(contentDisposition) { return contentDisposition } @@ -82,11 +99,11 @@ func safeContentDisposition(contentType string, contentDisposition string) strin func attachmentDisposition(contentDisposition string) string { if contentDisposition == "" { - return "attachment" + return attachmentDispositionText } - if InlineRegex.MatchString(contentDisposition) { - return InlineRegex.ReplaceAllString(contentDisposition, "attachment") + if inlineRegex.MatchString(contentDisposition) { + return inlineRegex.ReplaceAllString(contentDisposition, attachmentDispositionText) } return contentDisposition @@ -94,11 +111,11 @@ func attachmentDisposition(contentDisposition string) string { func inlineDisposition(contentDisposition string) string { if contentDisposition == "" { - return "inline" + return inlineDispositionText } - if AttachmentRegex.MatchString(contentDisposition) { - return AttachmentRegex.ReplaceAllString(contentDisposition, "inline") + if attachmentRegex.MatchString(contentDisposition) { + return attachmentRegex.ReplaceAllString(contentDisposition, inlineDispositionText) } return contentDisposition diff --git a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go index 2396bb0f952..b009cda1a24 100644 --- a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go +++ b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go @@ -56,11 +56,17 @@ func TestSetProperContentTypeAndDisposition(t *testing.T) { body: "Hello world!", }, { - desc: "Javascript type", + desc: "Javascript within HTML type", contentType: "text/plain; charset=utf-8", contentDisposition: "inline", body: "", }, + { + desc: "Javascript type", + contentType: "text/plain; charset=utf-8", + contentDisposition: "inline", + body: "alert(\"foo\")", + }, { desc: "Image type", contentType: "image/png", @@ -170,25 +176,41 @@ func TestSetProperContentTypeAndDisposition(t *testing.T) { } func TestFailOverrideContentType(t *testing.T) { - testCase := struct { - contentType string - body string + testCases := []struct { + desc string + overrideFromUpstream string + responseContentType string + body string }{ - contentType: "text/plain; charset=utf-8", - body: "Hello world!", + { + desc: "Force text/html into text/plain", + responseContentType: "text/plain; charset=utf-8", + overrideFromUpstream: "text/html; charset=utf-8", + body: "Hello world!", + }, + { + desc: "Force application/javascript into text/plain", + responseContentType: "text/plain; charset=utf-8", + overrideFromUpstream: "application/javascript; charset=utf-8", + body: "alert(1);", + }, } - h := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - // We are pretending to be upstream or an inner layer of the ResponseWriter chain - w.Header().Set(headers.GitlabWorkhorseDetectContentTypeHeader, "true") - w.Header().Set(headers.ContentTypeHeader, "text/html; charset=utf-8") - _, err := io.WriteString(w, testCase.body) - require.NoError(t, err) - }) + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + h := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + // We are pretending to be upstream or an inner layer of the ResponseWriter chain + w.Header().Set(headers.GitlabWorkhorseDetectContentTypeHeader, "true") + w.Header().Set(headers.ContentTypeHeader, tc.overrideFromUpstream) + _, err := io.WriteString(w, tc.body) + require.NoError(t, err) + }) - resp := makeRequest(t, h, testCase.body, "") + resp := makeRequest(t, h, tc.body, "") - require.Equal(t, testCase.contentType, resp.Header.Get(headers.ContentTypeHeader)) + require.Equal(t, tc.responseContentType, resp.Header.Get(headers.ContentTypeHeader)) + }) + } } func TestSuccessOverrideContentDispositionFromInlineToAttachment(t *testing.T) {