From b2cb8c48c5dddbbce803db8b7e600f722658002c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 10 Jul 2020 00:09:13 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- Gemfile | 2 - Gemfile.lock | 3 - .../javascripts/pages/shared/wikis/wikis.js | 19 +++ app/assets/javascripts/user_popovers.js | 3 + .../components/user_popover/user_popover.vue | 2 +- app/assets/stylesheets/framework/forms.scss | 4 - .../integrations/jira/issues_finder.rb | 8 +- app/helpers/wiki_helper.rb | 9 ++ app/models/user.rb | 12 +- app/models/user_detail.rb | 18 +++ app/views/shared/wikis/show.html.haml | 2 +- app/views/users/show.html.haml | 3 +- .../216199-track-wiki-page-views.yml | 5 + .../add-markdown-support-in-bio.yml | 5 + changelogs/unreleased/swap-to-oj.yml | 5 - config/initializers/multi_json.rb | 5 - config/initializers/oj.rb | 4 - config/locales/en.yml | 2 + ...0630091656_add_bio_html_to_user_details.rb | 24 ++++ db/structure.sql | 5 +- doc/api/users.md | 6 + doc/development/chatops_on_gitlabcom.md | 1 + doc/operations/index.md | 6 + doc/operations/metrics/index.md | 22 ++++ doc/user/project/integrations/prometheus.md | 8 -- lib/api/entities/user.rb | 2 +- lib/api/helpers.rb | 6 +- lib/api/helpers/users_helpers.rb | 7 ++ lib/api/users.rb | 2 + lib/gitlab/json.rb | 115 +++--------------- lib/gitlab/json_logger.rb | 2 +- locale/gitlab.pot | 9 -- .../profiles/user_edit_profile_spec.rb | 5 +- .../integrations/jira/issues_finder_spec.rb | 50 +++++--- .../user_popover/user_popover_spec.js | 11 +- spec/frontend/wikis_spec.js | 30 +++++ spec/helpers/wiki_helper_spec.rb | 20 +++ .../ci/parsers/accessibility/pa11y_spec.rb | 4 +- .../conduit/response_spec.rb | 2 +- spec/models/user_detail_spec.rb | 31 ++++- spec/models/user_spec.rb | 72 +++-------- 41 files changed, 317 insertions(+), 234 deletions(-) create mode 100644 changelogs/unreleased/216199-track-wiki-page-views.yml create mode 100644 changelogs/unreleased/add-markdown-support-in-bio.yml delete mode 100644 changelogs/unreleased/swap-to-oj.yml delete mode 100644 config/initializers/multi_json.rb delete mode 100644 config/initializers/oj.rb create mode 100644 db/migrate/20200630091656_add_bio_html_to_user_details.rb create mode 100644 doc/operations/metrics/index.md diff --git a/Gemfile b/Gemfile index 2d5b8c3cdb5..62d6d3767ed 100644 --- a/Gemfile +++ b/Gemfile @@ -500,5 +500,3 @@ gem 'valid_email', '~> 0.1' # JSON gem 'json', '~> 2.3.0' gem 'json-schema', '~> 2.8.0' -gem 'oj', '~> 3.10.6' -gem 'multi_json', '~> 1.14.1' diff --git a/Gemfile.lock b/Gemfile.lock index a14b657a983..f68fd455352 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -687,7 +687,6 @@ GEM octokit (4.15.0) faraday (>= 0.9) sawyer (~> 0.8.0, >= 0.5.3) - oj (3.10.6) omniauth (1.9.0) hashie (>= 3.4.6, < 3.7.0) rack (>= 1.6.2, < 3) @@ -1313,7 +1312,6 @@ DEPENDENCIES mimemagic (~> 0.3.2) mini_magick minitest (~> 5.11.0) - multi_json (~> 1.14.1) nakayoshi_fork (~> 0.0.4) net-ldap net-ntp @@ -1321,7 +1319,6 @@ DEPENDENCIES nokogiri (~> 1.10.9) oauth2 (~> 1.4) octokit (~> 4.15) - oj (~> 3.10.6) omniauth (~> 1.8) omniauth-auth0 (~> 2.0.0) omniauth-authentiq (~> 0.3.3) diff --git a/app/assets/javascripts/pages/shared/wikis/wikis.js b/app/assets/javascripts/pages/shared/wikis/wikis.js index ed67219383b..41d43812b5d 100644 --- a/app/assets/javascripts/pages/shared/wikis/wikis.js +++ b/app/assets/javascripts/pages/shared/wikis/wikis.js @@ -1,5 +1,6 @@ import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { s__, sprintf } from '~/locale'; +import Tracking from '~/tracking'; const MARKDOWN_LINK_TEXT = { markdown: '[Link Title](page-slug)', @@ -8,6 +9,9 @@ const MARKDOWN_LINK_TEXT = { org: '[[page-slug]]', }; +const TRACKING_EVENT_NAME = 'view_wiki_page'; +const TRACKING_CONTEXT_SCHEMA = 'iglu:com.gitlab/wiki_page_context/jsonschema/1-0-0'; + export default class Wikis { constructor() { this.sidebarEl = document.querySelector('.js-wiki-sidebar'); @@ -57,6 +61,8 @@ export default class Wikis { window.onbeforeunload = null; }); } + + Wikis.trackPageView(); } handleWikiTitleChange(e) { @@ -97,4 +103,17 @@ export default class Wikis { classList.remove('right-sidebar-expanded'); } } + + static trackPageView() { + const wikiPageContent = document.querySelector('.js-wiki-page-content[data-tracking-context]'); + if (!wikiPageContent) return; + + Tracking.event(document.body.dataset.page, TRACKING_EVENT_NAME, { + label: TRACKING_EVENT_NAME, + context: { + schema: TRACKING_CONTEXT_SCHEMA, + data: JSON.parse(wikiPageContent.dataset.trackingContext), + }, + }); + } } diff --git a/app/assets/javascripts/user_popovers.js b/app/assets/javascripts/user_popovers.js index bde00d72620..290de55e6f9 100644 --- a/app/assets/javascripts/user_popovers.js +++ b/app/assets/javascripts/user_popovers.js @@ -1,5 +1,7 @@ import Vue from 'vue'; +import sanitize from 'sanitize-html'; + import UsersCache from './lib/utils/users_cache'; import UserPopover from './vue_shared/components/user_popover/user_popover.vue'; @@ -38,6 +40,7 @@ const populateUserInfo = user => { name: userData.name, location: userData.location, bio: userData.bio, + bioHtml: sanitize(userData.bio_html), workInformation: userData.work_information, loaded: true, }); diff --git a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue index 19c9c70ecf1..bd35d3fead9 100644 --- a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue +++ b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue @@ -75,7 +75,7 @@ export default {
- {{ user.bio }} +
diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss index eebd3ce654e..ec8d5806345 100644 --- a/app/assets/stylesheets/framework/forms.scss +++ b/app/assets/stylesheets/framework/forms.scss @@ -128,10 +128,6 @@ label { display: inline; } -.wiki-content { - margin-top: 35px; -} - .form-control::placeholder { color: $gl-text-color-tertiary; } diff --git a/app/finders/projects/integrations/jira/issues_finder.rb b/app/finders/projects/integrations/jira/issues_finder.rb index 8bfc7729398..b156c879c4f 100644 --- a/app/finders/projects/integrations/jira/issues_finder.rb +++ b/app/finders/projects/integrations/jira/issues_finder.rb @@ -55,8 +55,14 @@ module Projects def map_sort_values(sort) case sort - when 'created_date' + when 'created_date', 'created_desc' { sort: 'created', sort_direction: 'DESC' } + when 'created_asc' + { sort: 'created', sort_direction: 'ASC' } + when 'updated_desc' + { sort: 'updated', sort_direction: 'DESC' } + when 'updated_asc' + { sort: 'updated', sort_direction: 'ASC' } else { sort: ::Jira::JqlBuilderService::DEFAULT_SORT, sort_direction: ::Jira::JqlBuilderService::DEFAULT_SORT_DIRECTION } end diff --git a/app/helpers/wiki_helper.rb b/app/helpers/wiki_helper.rb index c3ef4cbf841..c51dfefc0de 100644 --- a/app/helpers/wiki_helper.rb +++ b/app/helpers/wiki_helper.rb @@ -128,4 +128,13 @@ module WikiHelper raise NotImplementedError, "Unknown wiki container type #{wiki.container.class.name}" end end + + def wiki_page_tracking_context(page) + { + 'wiki-format' => page.format, + 'wiki-title-size' => page.title.bytesize, + 'wiki-content-size' => page.raw_content.bytesize, + 'wiki-directory-nest-level' => page.path.scan('/').count + } + end end diff --git a/app/models/user.rb b/app/models/user.rb index b744ad08d13..cee76ee7cfa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -69,6 +69,8 @@ class User < ApplicationRecord MINIMUM_INACTIVE_DAYS = 180 + ignore_column :bio, remove_with: '13.4', remove_after: '2020-09-22' + # Override Devise::Models::Trackable#update_tracked_fields! # to limit database writes to at most once every hour # rubocop: disable CodeReuse/ServiceClass @@ -193,7 +195,6 @@ class User < ApplicationRecord validates :notification_email, devise_email: true, if: ->(user) { user.notification_email != user.email } validates :public_email, presence: true, uniqueness: true, devise_email: true, allow_blank: true validates :commit_email, devise_email: true, allow_nil: true, if: ->(user) { user.commit_email != user.email } - validates :bio, length: { maximum: 255 }, allow_blank: true validates :projects_limit, presence: true, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: Gitlab::Database::MAX_INT_VALUE } @@ -228,7 +229,6 @@ class User < ApplicationRecord before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? } before_validation :ensure_namespace_correct before_save :ensure_namespace_correct # in case validation is skipped - before_save :ensure_bio_is_assigned_to_user_details, if: :bio_changed? after_validation :set_username_errors after_update :username_changed_hook, if: :saved_change_to_username? after_destroy :post_destroy_hook @@ -280,6 +280,7 @@ class User < ApplicationRecord delegate :path, to: :namespace, allow_nil: true, prefix: true delegate :job_title, :job_title=, to: :user_detail, allow_nil: true + delegate :bio, :bio=, :bio_html, to: :user_detail, allow_nil: true accepts_nested_attributes_for :user_preference, update_only: true accepts_nested_attributes_for :user_detail, update_only: true @@ -1272,13 +1273,6 @@ class User < ApplicationRecord end end - # Temporary, will be removed when bio is fully migrated - def ensure_bio_is_assigned_to_user_details - return if Feature.disabled?(:migrate_bio_to_user_details, default_enabled: true) - - user_detail.bio = bio.to_s[0...255] # bio can be NULL in users, but cannot be NULL in user_details - end - def set_username_errors namespace_path_errors = self.errors.delete(:"namespace.path") self.errors[:username].concat(namespace_path_errors) if namespace_path_errors diff --git a/app/models/user_detail.rb b/app/models/user_detail.rb index 5dc74421705..09bb2e080e5 100644 --- a/app/models/user_detail.rb +++ b/app/models/user_detail.rb @@ -1,7 +1,25 @@ # frozen_string_literal: true class UserDetail < ApplicationRecord + extend ::Gitlab::Utils::Override + include CacheMarkdownField + belongs_to :user validates :job_title, length: { maximum: 200 } + validates :bio, length: { maximum: 255 }, allow_blank: true + + cache_markdown_field :bio + + def bio_html + read_attribute(:bio_html) || bio + end + + # For backward compatibility. + # Older migrations (and their tests) reference the `User.migration_bot` where the `bio` attribute is set. + # Here we disable writing the markdown cache when the `bio_html` column does not exists. + override :invalidated_markdown_cache? + def invalidated_markdown_cache? + self.class.column_names.include?('bio_html') && super + end end diff --git a/app/views/shared/wikis/show.html.haml b/app/views/shared/wikis/show.html.haml index 9f016647690..a7c734f5af4 100644 --- a/app/views/shared/wikis/show.html.haml +++ b/app/views/shared/wikis/show.html.haml @@ -21,7 +21,7 @@ = (s_("WikiHistoricalPage|You can view the %{most_recent_link} or browse the %{history_link}.") % { most_recent_link: most_recent_link, history_link: history_link }).html_safe .gl-mt-3.gl-mb-3 - .md{ data: { qa_selector: 'wiki_page_content' } } + .js-wiki-page-content.md{ data: { qa_selector: 'wiki_page_content', tracking_context: wiki_page_tracking_context(@page).to_json } } = render_wiki_content(@page) = render 'shared/wikis/sidebar' diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 85d8e584eba..d2f7ff91f0d 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -85,7 +85,8 @@ - if @user.bio.present? .cover-desc.cgray %p.profile-user-bio - = @user.bio + = markdown(@user.bio_html) + - unless profile_tabs.empty? .scrolling-tabs-container diff --git a/changelogs/unreleased/216199-track-wiki-page-views.yml b/changelogs/unreleased/216199-track-wiki-page-views.yml new file mode 100644 index 00000000000..aec95777ebf --- /dev/null +++ b/changelogs/unreleased/216199-track-wiki-page-views.yml @@ -0,0 +1,5 @@ +--- +title: Track wiki page views in Snowplow +merge_request: 35784 +author: +type: changed diff --git a/changelogs/unreleased/add-markdown-support-in-bio.yml b/changelogs/unreleased/add-markdown-support-in-bio.yml new file mode 100644 index 00000000000..a22ee193bda --- /dev/null +++ b/changelogs/unreleased/add-markdown-support-in-bio.yml @@ -0,0 +1,5 @@ +--- +title: Add support for Markdown in the user's bio +merge_request: 35604 +author: Riccardo Padovani +type: added diff --git a/changelogs/unreleased/swap-to-oj.yml b/changelogs/unreleased/swap-to-oj.yml deleted file mode 100644 index c0193ebd9e9..00000000000 --- a/changelogs/unreleased/swap-to-oj.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Add oj gem for faster JSON -merge_request: 35527 -author: -type: performance diff --git a/config/initializers/multi_json.rb b/config/initializers/multi_json.rb deleted file mode 100644 index 93a81d8320d..00000000000 --- a/config/initializers/multi_json.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -# Explicitly set the JSON adapter used by MultiJson -# Currently we want this to default to the existing json gem -MultiJson.use(:json_gem) diff --git a/config/initializers/oj.rb b/config/initializers/oj.rb deleted file mode 100644 index 3fa26259fc6..00000000000 --- a/config/initializers/oj.rb +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -# Ensure Oj runs in json-gem compatibility mode by default -Oj.default_options = { mode: :rails } diff --git a/config/locales/en.yml b/config/locales/en.yml index ed0552ab452..7ff4e3bf7da 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -22,6 +22,8 @@ en: grafana_enabled: "Grafana integration enabled" user/user_detail: job_title: 'Job title' + user/user_detail: + bio: 'Bio' views: pagination: previous: "Prev" diff --git a/db/migrate/20200630091656_add_bio_html_to_user_details.rb b/db/migrate/20200630091656_add_bio_html_to_user_details.rb new file mode 100644 index 00000000000..6a9df85d6a4 --- /dev/null +++ b/db/migrate/20200630091656_add_bio_html_to_user_details.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class AddBioHtmlToUserDetails < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + with_lock_retries do + # Note: bio_html is calculated from bio, the bio column is already constrained + add_column :user_details, :bio_html, :text # rubocop:disable Migration/AddLimitToTextColumns + add_column :user_details, :cached_markdown_version, :integer + end + end + + def down + with_lock_retries do + remove_column :user_details, :bio_html + remove_column :user_details, :cached_markdown_version + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 1804ff157c0..9c48e97aaf7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15640,7 +15640,9 @@ ALTER SEQUENCE public.user_custom_attributes_id_seq OWNED BY public.user_custom_ CREATE TABLE public.user_details ( user_id bigint NOT NULL, job_title character varying(200) DEFAULT ''::character varying NOT NULL, - bio character varying(255) DEFAULT ''::character varying NOT NULL + bio character varying(255) DEFAULT ''::character varying NOT NULL, + bio_html text, + cached_markdown_version integer ); CREATE SEQUENCE public.user_details_user_id_seq @@ -23635,6 +23637,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200626060151 20200626130220 20200629192638 +20200630091656 20200630110826 20200701093859 20200702123805 diff --git a/doc/api/users.md b/doc/api/users.md index b5551beff9f..15f99367b2e 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -90,6 +90,7 @@ GET /users "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, + "bio_html": null, "location": null, "skype": "", "linkedin": "", @@ -129,6 +130,7 @@ GET /users "created_at": "2012-05-23T08:01:01Z", "is_admin": false, "bio": null, + "bio_html": null, "location": null, "skype": "", "linkedin": "", @@ -246,6 +248,7 @@ Parameters: "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", "bio": null, + "bio_html": null, "location": null, "public_email": "john@example.com", "skype": "", @@ -281,6 +284,7 @@ Example Responses: "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, + "bio_html": null, "location": null, "public_email": "john@example.com", "skype": "", @@ -500,6 +504,7 @@ GET /user "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", "bio": null, + "bio_html": null, "location": null, "public_email": "john@example.com", "skype": "", @@ -549,6 +554,7 @@ GET /user "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, + "bio_html": null, "location": null, "public_email": "john@example.com", "skype": "", diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index 60056c2f65c..33c5e7ed3ce 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -14,6 +14,7 @@ tasks such as: To request access to Chatops on GitLab.com: 1. Log into **using the same username** as for GitLab.com (you may have to rename it). + 1. You could also use the "Sign in with" Google button to sign in, with your GitLab.com email address. 1. Ask in the [#production](https://gitlab.slack.com/messages/production) channel for an existing member to add you to the `chatops` project in Ops. They can do it by running `/chatops run member add gitlab-com/chatops --ops` command in that channel. NOTE: **Note:** If you had to change your username for GitLab.com on the first step, make sure [to reflect this information](https://gitlab.com/gitlab-com/www-gitlab-com#adding-yourself-to-the-team-page) on [the team page](https://about.gitlab.com/company/team/). diff --git a/doc/operations/index.md b/doc/operations/index.md index 0faa22bae79..95225514590 100644 --- a/doc/operations/index.md +++ b/doc/operations/index.md @@ -1,3 +1,9 @@ +--- +stage: Monitor +group: APM +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- + # Project operations GitLab provides a variety of tools to help operate and maintain diff --git a/doc/operations/metrics/index.md b/doc/operations/metrics/index.md new file mode 100644 index 00000000000..7f9e8fa03e1 --- /dev/null +++ b/doc/operations/metrics/index.md @@ -0,0 +1,22 @@ +--- +stage: Monitor +group: APM +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- + +# Monitor CI/CD Environment metrics + +Once [configured](../../user/project/integrations/prometheus.md), GitLab will attempt to retrieve performance metrics for any +environment which has had a successful deployment. + +GitLab will automatically scan the Prometheus server for metrics from known servers like Kubernetes and NGINX, and attempt to identify individual environments. The supported metrics and scan process is detailed in our [Prometheus Metrics Library documentation](../../user/project/integrations/prometheus_library/index.md). + +You can view the performance dashboard for an environment by [clicking on the monitoring button](../../ci/environments/index.md#monitoring-environments). + +## Metrics dashboard visibility + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/201924) in GitLab 13.0. + +You can set the visibility of the metrics dashboard to **Only Project Members** +or **Everyone With Access**. When set to **Everyone with Access**, the metrics +dashboard is visible to authenticated and non-authenticated users. diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 4bf8f47092d..a6561505733 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -1270,14 +1270,6 @@ Prerequisites for embedding from a Grafana instance: 1. In GitLab, paste the URL into a Markdown field and save. The chart will take a few moments to render. ![GitLab Rendered Grafana Panel](img/rendered_grafana_embed_v12_5.png) -## Metrics dashboard visibility - -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/201924) in GitLab 13.0. - -You can set the visibility of the metrics dashboard to **Only Project Members** -or **Everyone With Access**. When set to **Everyone with Access**, the metrics -dashboard is visible to authenticated and non-authenticated users. - ## Troubleshooting When troubleshooting issues with a managed Prometheus app, it is often useful to diff --git a/lib/api/entities/user.rb b/lib/api/entities/user.rb index adf954ab02d..4aa5c9b7236 100644 --- a/lib/api/entities/user.rb +++ b/lib/api/entities/user.rb @@ -5,7 +5,7 @@ module API class User < UserBasic include UsersHelper expose :created_at, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } - expose :bio, :location, :public_email, :skype, :linkedin, :twitter, :website_url, :organization, :job_title + expose :bio, :bio_html, :location, :public_email, :skype, :linkedin, :twitter, :website_url, :organization, :job_title expose :work_information do |user| work_information(user) end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 101a8ed9a4b..056ab85e906 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -410,10 +410,14 @@ module API def render_validation_error!(model) if model.errors.any? - render_api_error!(model.errors.messages || '400 Bad Request', 400) + render_api_error!(model_error_messages(model) || '400 Bad Request', 400) end end + def model_error_messages(model) + model.errors.messages + end + def render_spam_error! render_api_error!({ error: 'Spam detected' }, 400) end diff --git a/lib/api/helpers/users_helpers.rb b/lib/api/helpers/users_helpers.rb index 99eefc1cbb9..2d7b22e66b3 100644 --- a/lib/api/helpers/users_helpers.rb +++ b/lib/api/helpers/users_helpers.rb @@ -11,6 +11,13 @@ module API params :optional_index_params_ee do end + + def model_error_messages(model) + super.tap do |error_messages| + # Remapping errors from nested associations. + error_messages[:bio] = error_messages.delete(:"user_detail.bio") if error_messages.has_key?(:"user_detail.bio") + end + end end end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 3124acd511f..7942777287b 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -117,6 +117,8 @@ module API users = users.preload(:identities, :u2f_registrations) if entity == Entities::UserWithAdmin users, options = with_custom_attributes(users, { with: entity, current_user: current_user }) + users = users.preload(:user_detail) + present paginate(users), options end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/gitlab/json.rb b/lib/gitlab/json.rb index 90de67d2bc4..5b6689dbefe 100644 --- a/lib/gitlab/json.rb +++ b/lib/gitlab/json.rb @@ -1,140 +1,59 @@ # frozen_string_literal: true -# This is a GitLab-specific JSON interface. You should use this instead -# of using `JSON` directly. This allows us to swap the adapter and handle -# legacy issues. - module Gitlab module Json INVALID_LEGACY_TYPES = [String, TrueClass, FalseClass].freeze class << self - # Parse a string and convert it to a Ruby object - # - # @param string [String] the JSON string to convert to Ruby objects - # @param opts [Hash] an options hash in the standard JSON gem format - # @return [Boolean, String, Array, Hash] - # @raise [JSON::ParserError] raised if parsing fails - def parse(string, opts = {}) - # First we should ensure this really is a string, not some other - # type which purports to be a string. This handles some legacy - # usage of the JSON class. - string = string.to_s unless string.is_a?(String) - - legacy_mode = legacy_mode_enabled?(opts.delete(:legacy_mode)) - data = adapter_load(string, opts) + def parse(string, *args, **named_args) + legacy_mode = legacy_mode_enabled?(named_args.delete(:legacy_mode)) + data = adapter.parse(string, *args, **named_args) handle_legacy_mode!(data) if legacy_mode data end - alias_method :parse!, :parse + def parse!(string, *args, **named_args) + legacy_mode = legacy_mode_enabled?(named_args.delete(:legacy_mode)) + data = adapter.parse!(string, *args, **named_args) - # Take a Ruby object and convert it to a string - # - # @param object [Boolean, String, Array, Hash, Object] depending on the adapter this can be a variety of types - # @param opts [Hash] an options hash in the standard JSON gem format - # @return [String] - def dump(object, opts = {}) - adapter_dump(object, opts) + handle_legacy_mode!(data) if legacy_mode + + data + end + + def dump(*args) + adapter.dump(*args) end - # Legacy method used in our codebase that might just be an alias for `parse`. - # Will be updated to use our `parse` method. def generate(*args) - ::JSON.generate(*args) + adapter.generate(*args) end - # Generates a JSON string and formats it nicely. - # Varies depending on adapter and will be updated to use our methods. def pretty_generate(*args) - ::JSON.pretty_generate(*args) + adapter.pretty_generate(*args) end private - # Convert JSON string into Ruby through toggleable adapters. - # - # Must rescue adapter-specific errors and return `parser_error`, and - # must also standardize the options hash to support each adapter as - # they all take different options. - # - # @param string [String] the JSON string to convert to Ruby objects - # @param opts [Hash] an options hash in the standard JSON gem format - # @return [Boolean, String, Array, Hash] - # @raise [JSON::ParserError] - def adapter_load(string, opts = {}) - opts = standardize_opts(opts) - - if enable_oj? - Oj.load(string, opts) - else - ::JSON.parse(string, opts) - end - rescue Oj::ParseError, Encoding::UndefinedConversionError => ex - raise parser_error.new(ex) + def adapter + ::JSON end - # Convert Ruby object to JSON string through toggleable adapters. - # - # @param object [Boolean, String, Array, Hash, Object] depending on the adapter this can be a variety of types - # @param opts [Hash] an options hash in the standard JSON gem format - # @return [String] - def adapter_dump(thing, opts = {}) - opts = standardize_opts(opts) - - if enable_oj? - Oj.dump(thing, opts) - else - ::JSON.dump(thing, opts) - end - end - - # Take a JSON standard options hash and standardize it to work across adapters - # An example of this is Oj taking :symbol_keys instead of :symbolize_names - # - # @param opts [Hash] - # @return [Hash] - def standardize_opts(opts = {}) - if enable_oj? - opts[:mode] = :rails - opts[:symbol_keys] = opts[:symbolize_keys] || opts[:symbolize_names] - end - - opts - end - - # The standard parser error we should be returning. Defined in a method - # so we can potentially override it later. - # - # @return [JSON::ParserError] def parser_error ::JSON::ParserError end - # @param [Nil, Boolean] an extracted :legacy_mode key from the opts hash - # @return [Boolean] def legacy_mode_enabled?(arg_value) arg_value.nil? ? false : arg_value end - # If legacy mode is enabled, we need to raise an error depending on the values - # provided in the string. This will be deprecated. - # - # @param data [Boolean, String, Array, Hash, Object] - # @return [Boolean, String, Array, Hash, Object] - # @raise [JSON::ParserError] def handle_legacy_mode!(data) return data unless Feature.enabled?(:json_wrapper_legacy_mode, default_enabled: true) raise parser_error if INVALID_LEGACY_TYPES.any? { |type| data.is_a?(type) } end - - # @return [Boolean] - def enable_oj? - Feature.enabled?(:oj_json, default_enabled: true) - end end end end diff --git a/lib/gitlab/json_logger.rb b/lib/gitlab/json_logger.rb index 3a74df8dc8f..ab34fb03158 100644 --- a/lib/gitlab/json_logger.rb +++ b/lib/gitlab/json_logger.rb @@ -19,7 +19,7 @@ module Gitlab data.merge!(message) end - Gitlab::Json.dump(data) + "\n" + data.to_json + "\n" end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a7d378b8eba..f2d0e372d88 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10453,9 +10453,6 @@ msgstr "" msgid "Geo Settings" msgstr "" -msgid "Geo allows you to replicate your GitLab instance to other geographical locations." -msgstr "" - msgid "Geo nodes are paused using a command run on the node" msgstr "" @@ -13735,9 +13732,6 @@ msgstr "" msgid "List available repositories" msgstr "" -msgid "List of IPs and CIDRs of allowed secondary nodes. Comma-separated, e.g. \"1.1.1.1, 2.2.2.0/24\"" -msgstr "" - msgid "List settings" msgstr "" @@ -22907,9 +22901,6 @@ msgstr "" msgid "The above settings apply to all projects with the selected compliance framework(s)." msgstr "" -msgid "The amount of seconds after which a request to get a secondary node status will time out." -msgstr "" - msgid "The application will be used where the client secret can be kept confidential. Native mobile apps and Single Page Apps are considered non-confidential." msgstr "" diff --git a/spec/features/profiles/user_edit_profile_spec.rb b/spec/features/profiles/user_edit_profile_spec.rb index 2659157d61d..697fccaca34 100644 --- a/spec/features/profiles/user_edit_profile_spec.rb +++ b/spec/features/profiles/user_edit_profile_spec.rb @@ -26,7 +26,7 @@ RSpec.describe 'User edit profile' do fill_in 'user_twitter', with: 'testtwitter' fill_in 'user_website_url', with: 'testurl' fill_in 'user_location', with: 'Ukraine' - fill_in 'user_bio', with: 'I <3 GitLab' + fill_in 'user_bio', with: 'I <3 GitLab :tada:' fill_in 'user_job_title', with: 'Frontend Engineer' fill_in 'user_organization', with: 'GitLab' submit_settings @@ -36,7 +36,8 @@ RSpec.describe 'User edit profile' do linkedin: 'testlinkedin', twitter: 'testtwitter', website_url: 'testurl', - bio: 'I <3 GitLab', + bio: 'I <3 GitLab :tada:', + bio_html: '

I <3 GitLab 🎉

', job_title: 'Frontend Engineer', organization: 'GitLab' ) diff --git a/spec/finders/projects/integrations/jira/issues_finder_spec.rb b/spec/finders/projects/integrations/jira/issues_finder_spec.rb index ea17778f7ba..fd55585cc3b 100644 --- a/spec/finders/projects/integrations/jira/issues_finder_spec.rb +++ b/spec/finders/projects/integrations/jira/issues_finder_spec.rb @@ -76,27 +76,45 @@ RSpec.describe Projects::Integrations::Jira::IssuesFinder do expect(issues.map(&:key)).to eq(%w[TEST-1 TEST-2]) end - context 'when sort by created_date' do - let(:params) { { sort: 'created_date' } } + context 'when sorting' do + shared_examples 'maps sort values' do + it do + expect(::Jira::JqlBuilderService).to receive(:new) + .with(jira_service.project_key, expected_sort_values) + .and_call_original - it 'maps sort correctly' do - expect(::Jira::JqlBuilderService).to receive(:new) - .with(jira_service.project_key, { sort: 'created', sort_direction: 'DESC' }) - .and_call_original - - subject + subject + end end - end - context 'when sort by unknown_sort' do - let(:params) { { sort: 'unknown_sort' } } + it_behaves_like 'maps sort values' do + let(:params) { { sort: 'created_date' } } + let(:expected_sort_values) { { sort: 'created', sort_direction: 'DESC' } } + end - it 'maps sort to default' do - expect(::Jira::JqlBuilderService).to receive(:new) - .with(jira_service.project_key, { sort: 'created', sort_direction: 'DESC' }) - .and_call_original + it_behaves_like 'maps sort values' do + let(:params) { { sort: 'created_desc' } } + let(:expected_sort_values) { { sort: 'created', sort_direction: 'DESC' } } + end - subject + it_behaves_like 'maps sort values' do + let(:params) { { sort: 'created_asc' } } + let(:expected_sort_values) { { sort: 'created', sort_direction: 'ASC' } } + end + + it_behaves_like 'maps sort values' do + let(:params) { { sort: 'updated_desc' } } + let(:expected_sort_values) { { sort: 'updated', sort_direction: 'DESC' } } + end + + it_behaves_like 'maps sort values' do + let(:params) { { sort: 'updated_asc' } } + let(:expected_sort_values) { { sort: 'updated', sort_direction: 'ASC' } } + end + + it_behaves_like 'maps sort values' do + let(:params) { { sort: 'unknown_sort' } } + let(:expected_sort_values) { { sort: 'created', sort_direction: 'DESC' } } end end diff --git a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js index 16aa8379f56..a4ff6ac0c16 100644 --- a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js +++ b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js @@ -83,9 +83,10 @@ describe('User Popover Component', () => { describe('job data', () => { const findWorkInformation = () => wrapper.find({ ref: 'workInformation' }); const findBio = () => wrapper.find({ ref: 'bio' }); + const bio = 'My super interesting bio'; it('should show only bio if work information is not available', () => { - const user = { ...DEFAULT_PROPS.user, bio: 'My super interesting bio' }; + const user = { ...DEFAULT_PROPS.user, bio, bioHtml: bio }; createWrapper({ user }); @@ -107,7 +108,8 @@ describe('User Popover Component', () => { it('should display bio and work information in separate lines', () => { const user = { ...DEFAULT_PROPS.user, - bio: 'My super interesting bio', + bio, + bioHtml: bio, workInformation: 'Frontend Engineer at GitLab', }; @@ -120,12 +122,13 @@ describe('User Popover Component', () => { it('should not encode special characters in bio', () => { const user = { ...DEFAULT_PROPS.user, - bio: 'I like & CSS', + bio: 'I like CSS', + bioHtml: 'I like CSS', }; createWrapper({ user }); - expect(findBio().text()).toBe('I like & CSS'); + expect(findBio().html()).toContain('I like CSS'); }); it('shows icon for bio', () => { diff --git a/spec/frontend/wikis_spec.js b/spec/frontend/wikis_spec.js index 8c68edafd16..3469be4da1c 100644 --- a/spec/frontend/wikis_spec.js +++ b/spec/frontend/wikis_spec.js @@ -1,4 +1,6 @@ +import { escape } from 'lodash'; import Wikis from '~/pages/shared/wikis/wikis'; +import Tracking from '~/tracking'; import { setHTMLFixture } from './helpers/fixtures'; describe('Wikis', () => { @@ -122,4 +124,32 @@ describe('Wikis', () => { }); }); }); + + describe('trackPageView', () => { + const trackingPage = 'projects:wikis:show'; + const trackingContext = { foo: 'bar' }; + const showPageHtmlFixture = ` +
+ `; + + beforeEach(() => { + setHTMLFixture(showPageHtmlFixture); + document.body.dataset.page = trackingPage; + jest.spyOn(Tracking, 'event').mockImplementation(); + + Wikis.trackPageView(); + }); + + it('sends the tracking event and context', () => { + expect(Tracking.event).toHaveBeenCalledWith(trackingPage, 'view_wiki_page', { + label: 'view_wiki_page', + context: { + schema: 'iglu:com.gitlab/wiki_page_context/jsonschema/1-0-0', + data: trackingContext, + }, + }); + }); + }); }); diff --git a/spec/helpers/wiki_helper_spec.rb b/spec/helpers/wiki_helper_spec.rb index a2c4f02278d..040368b5ebd 100644 --- a/spec/helpers/wiki_helper_spec.rb +++ b/spec/helpers/wiki_helper_spec.rb @@ -104,4 +104,24 @@ RSpec.describe WikiHelper do expect(helper.wiki_sort_title('unknown')).to eq('Title') end end + + describe '#wiki_page_tracking_context' do + let_it_be(:page) { create(:wiki_page, title: 'path/to/page 💩', content: '💩', format: :markdown) } + + subject { helper.wiki_page_tracking_context(page) } + + it 'returns the tracking context' do + expect(subject).to eq( + 'wiki-format' => :markdown, + 'wiki-title-size' => 9, + 'wiki-content-size' => 4, + 'wiki-directory-nest-level' => 2 + ) + end + + it 'returns a nest level of zero for toplevel files' do + expect(page).to receive(:path).and_return('page') + expect(subject).to include('wiki-directory-nest-level' => 0) + end + end end diff --git a/spec/lib/gitlab/ci/parsers/accessibility/pa11y_spec.rb b/spec/lib/gitlab/ci/parsers/accessibility/pa11y_spec.rb index b3edf452f36..dda61bd5f6b 100644 --- a/spec/lib/gitlab/ci/parsers/accessibility/pa11y_spec.rb +++ b/spec/lib/gitlab/ci/parsers/accessibility/pa11y_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' RSpec.describe Gitlab::Ci::Parsers::Accessibility::Pa11y do describe '#parse!' do @@ -108,7 +108,7 @@ RSpec.describe Gitlab::Ci::Parsers::Accessibility::Pa11y do it "sets error_message" do expect { subject }.not_to raise_error - expect(accessibility_report.error_message).to include('JSON parsing failed') + expect(accessibility_report.error_message).to include('Pa11y parsing failed') expect(accessibility_report.errors_count).to eq(0) expect(accessibility_report.passes_count).to eq(0) expect(accessibility_report.scans_count).to eq(0) diff --git a/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb index c368b349a3c..2788c7e595b 100644 --- a/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb +++ b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Gitlab::PhabricatorImport::Conduit::Response do body: 'This is no JSON') expect { described_class.parse!(fake_response) } - .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /unexpected character/) + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /unexpected token at/) end it 'returns a parsed response for valid input' do diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index 407b82a7ac3..041af5b9c31 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -6,9 +6,38 @@ RSpec.describe UserDetail do it { is_expected.to belong_to(:user) } describe 'validations' do - describe 'job_title' do + describe '#job_title' do it { is_expected.not_to validate_presence_of(:job_title) } it { is_expected.to validate_length_of(:job_title).is_at_most(200) } end + + describe '#bio' do + it { is_expected.to validate_length_of(:bio).is_at_most(255) } + end + end + + describe '#bio_html' do + let(:user) { create(:user, bio: 'some **bio**') } + + subject { user.user_detail.bio_html } + + it 'falls back to #bio when the html representation is missing' do + user.user_detail.update!(bio_html: nil) + + expect(subject).to eq(user.user_detail.bio) + end + + it 'stores rendered html' do + expect(subject).to include('some bio') + end + + it 'does not try to set the value when the column is not there' do + without_bio_html_column = UserDetail.column_names - ['bio_html'] + + expect(described_class).to receive(:column_names).at_least(:once).and_return(without_bio_html_column) + expect(user.user_detail).not_to receive(:bio_html=) + + subject + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 1c122a096f7..69e15c22897 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -58,6 +58,10 @@ RSpec.describe User do 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 } + + it { is_expected.to delegate_method(:bio).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:bio=).to(:user_detail).with_arguments(:args).allow_nil } + it { is_expected.to delegate_method(:bio_html).to(:user_detail).allow_nil } end describe 'associations' do @@ -91,64 +95,28 @@ RSpec.describe User do it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:user) } it { is_expected.to have_many(:reviews).inverse_of(:author) } - describe "#bio" do - it 'syncs bio with `user_details.bio` on create' do + describe "#user_detail" do + it 'does not persist `user_detail` by default' do + expect(create(:user).user_detail).not_to be_persisted + end + + it 'creates `user_detail` when `bio` is given' do + user = create(:user, bio: 'my bio') + + expect(user.user_detail).to be_persisted + expect(user.user_detail.bio).to eq('my bio') + end + + it 'delegates `bio` to `user_detail`' do user = create(:user, bio: 'my bio') expect(user.bio).to eq(user.user_detail.bio) end - context 'when `migrate_bio_to_user_details` feature flag is off' do - before do - stub_feature_flags(migrate_bio_to_user_details: false) - end - - it 'does not sync bio with `user_details.bio`' do - user = create(:user, bio: 'my bio') - - expect(user.bio).to eq('my bio') - expect(user.user_detail.bio).to eq('') - end - end - - it 'syncs bio with `user_details.bio` on update' do + it 'creates `user_detail` when `bio` is first updated' do user = create(:user) - user.update!(bio: 'my bio') - - expect(user.bio).to eq(user.user_detail.bio) - end - - context 'when `user_details` association already exists' do - let(:user) { create(:user) } - - before do - create(:user_detail, user: user) - end - - it 'syncs bio with `user_details.bio`' do - user.update!(bio: 'my bio') - - expect(user.bio).to eq(user.user_detail.bio) - end - - it 'falls back to "" when nil is given' do - user.update!(bio: nil) - - expect(user.bio).to eq(nil) - expect(user.user_detail.bio).to eq('') - end - - # very unlikely scenario - it 'truncates long bio when syncing to user_details' do - invalid_bio = 'a' * 256 - truncated_bio = 'a' * 255 - - user.bio = invalid_bio - user.save(validate: false) - - expect(user.user_detail.bio).to eq(truncated_bio) - end + expect { user.update(bio: 'my bio') }.to change { user.user_detail.persisted? }.from(false).to(true) end end @@ -337,8 +305,6 @@ RSpec.describe User do it { is_expected.not_to allow_value(-1).for(:projects_limit) } it { is_expected.not_to allow_value(Gitlab::Database::MAX_INT_VALUE + 1).for(:projects_limit) } - it { is_expected.to validate_length_of(:bio).is_at_most(255) } - it_behaves_like 'an object with email-formated attributes', :email do subject { build(:user) } end