From 652bd073731b0028641672a75355c7918b5ac116 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 19 Mar 2020 12:09:33 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../logs/components/environment_logs.vue | 84 ++-- app/assets/stylesheets/pages/builds.scss | 2 +- app/models/event.rb | 31 +- app/models/project_wiki.rb | 9 +- app/models/wiki_page.rb | 13 + app/models/wiki_page/meta.rb | 136 +++++++ app/models/wiki_page/slug.rb | 26 ++ app/policies/wiki_page/meta_policy.rb | 5 + .../metrics/dashboard/clone_dashboard_service.rb | 2 +- .../dashboard/self_monitoring_dashboard_service.rb | 2 +- .../metrics/dashboard/system_dashboard_service.rb | 4 +- .../119208-limit-metric-type-on-list-pd.yml | 5 + ...enting-filtered-search-dropdown-improvement.yml | 5 + .../208234-optimize-ldap-keys-in-usage-data.yml | 5 + .../unreleased/30526-a-be-wiki-activity-Models.yml | 5 + config/sidekiq_queues.yml | 2 +- ...20200219135440_add_limit_metric_type_to_list.rb | 9 + db/migrate/20200302152516_add_wiki_slug.rb | 22 ++ ...6111759_add_index_on_id_and_ldap_key_to_keys.rb | 18 + db/schema.rb | 25 +- doc/development/architecture.md | 4 +- doc/development/changelog.md | 5 +- lib/gitlab/hook_data/wiki_page_builder.rb | 4 + .../stages/custom_metrics_details_inserter.rb | 40 ++ .../dashboard/stages/custom_metrics_inserter.rb | 107 +++++ .../stages/project_metrics_details_inserter.rb | 40 -- .../dashboard/stages/project_metrics_inserter.rb | 107 ----- locale/gitlab.pot | 13 +- spec/factories/events.rb | 10 + spec/factories/lists.rb | 1 + spec/factories/wiki_pages.rb | 56 ++- spec/fixtures/api/schemas/list.json | 3 +- .../logs/components/environment_logs_spec.js | 39 +- spec/graphql/types/base_field_spec.rb | 38 +- .../gitlab/import_export/safe_model_attributes.yml | 9 + .../lib/gitlab/metrics/dashboard/processor_spec.rb | 4 +- spec/models/event_spec.rb | 354 ++++++++++------- spec/models/wiki_page/meta_spec.rb | 430 +++++++++++++++++++++ spec/models/wiki_page/slug_spec.rb | 94 +++++ spec/models/wiki_page_spec.rb | 30 +- .../dashboard/clone_dashboard_service_spec.rb | 2 +- 41 files changed, 1439 insertions(+), 361 deletions(-) create mode 100644 app/models/wiki_page/meta.rb create mode 100644 app/models/wiki_page/slug.rb create mode 100644 app/policies/wiki_page/meta_policy.rb create mode 100644 changelogs/unreleased/119208-limit-metric-type-on-list-pd.yml create mode 100644 changelogs/unreleased/207912-implementing-filtered-search-dropdown-improvement.yml create mode 100644 changelogs/unreleased/208234-optimize-ldap-keys-in-usage-data.yml create mode 100644 changelogs/unreleased/30526-a-be-wiki-activity-Models.yml create mode 100644 db/migrate/20200219135440_add_limit_metric_type_to_list.rb create mode 100644 db/migrate/20200302152516_add_wiki_slug.rb create mode 100644 db/migrate/20200316111759_add_index_on_id_and_ldap_key_to_keys.rb create mode 100644 lib/gitlab/metrics/dashboard/stages/custom_metrics_details_inserter.rb create mode 100644 lib/gitlab/metrics/dashboard/stages/custom_metrics_inserter.rb delete mode 100644 lib/gitlab/metrics/dashboard/stages/project_metrics_details_inserter.rb delete mode 100644 lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb create mode 100644 spec/models/wiki_page/meta_spec.rb create mode 100644 spec/models/wiki_page/slug_spec.rb diff --git a/app/assets/javascripts/logs/components/environment_logs.vue b/app/assets/javascripts/logs/components/environment_logs.vue index 03019e4c25e..92d4be81c75 100644 --- a/app/assets/javascripts/logs/components/environment_logs.vue +++ b/app/assets/javascripts/logs/components/environment_logs.vue @@ -3,8 +3,10 @@ import { throttle } from 'lodash'; import { mapActions, mapState, mapGetters } from 'vuex'; import { GlSprintf, + GlIcon, GlAlert, GlDropdown, + GlDropdownHeader, GlDropdownDivider, GlDropdownItem, GlFormGroup, @@ -22,8 +24,10 @@ import { formatDate } from '../utils'; export default { components: { GlSprintf, + GlIcon, GlAlert, GlDropdown, + GlDropdownHeader, GlDropdownDivider, GlDropdownItem, GlFormGroup, @@ -124,6 +128,12 @@ export default { 'fetchMoreLogsPrepend', ]), + isCurrentEnvironment(envName) { + return envName === this.environments.current; + }, + isCurrentPod(podName) { + return podName === this.pods.current; + }, topReached() { if (!this.logs.isLoading) { this.fetchMoreLogsPrepend(); @@ -161,7 +171,6 @@ export default {
+ + {{ s__('Environments|Select environment') }} + - {{ env.name }} +
+ +
{{ env.name }}
+
+ + + {{ s__('Environments|Filter by pod') }} + + + + + + {{ s__('Environments|No pods to display') }} + + - {{ podName }} +
+ +
{{ podName }}
+
+ + + + - - -
{ reorder(id: :desc) } scope :code_push, -> { where(action: PUSHED) } scope :merged, -> { where(action: MERGED) } + scope :for_wiki_page, -> { where(target_type: WikiPage::Meta.name) } scope :with_associations, -> do # We're using preload for "push_event_payload" as otherwise the association @@ -197,6 +200,14 @@ class Event < ApplicationRecord created_action? && !target && target_type.nil? end + def created_wiki_page? + wiki_page? && action == CREATED + end + + def updated_wiki_page? + wiki_page? && action == UPDATED + end + def created_target? created_action? && target end @@ -217,6 +228,10 @@ class Event < ApplicationRecord target_type == "MergeRequest" end + def wiki_page? + target_type == WikiPage::Meta.name + end + def milestone target if milestone? end @@ -229,6 +244,14 @@ class Event < ApplicationRecord target if merge_request? end + def wiki_page + strong_memoize(:wiki_page) do + next unless wiki_page? + + ProjectWiki.new(project, author).find_page(target.canonical_slug) + end + end + def note target if note? end @@ -250,6 +273,10 @@ class Event < ApplicationRecord 'destroyed' elsif commented_action? "commented on" + elsif created_wiki_page? + 'created' + elsif updated_wiki_page? + 'updated' elsif created_project_action? created_project_action_name else @@ -362,6 +389,8 @@ class Event < ApplicationRecord :read_snippet elsif milestone? :read_milestone + elsif wiki_page? + :read_wiki end end end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index f8528a41634..50ce1c67453 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -19,10 +19,11 @@ class ProjectWiki DIRECTION_DESC = 'desc' DIRECTION_ASC = 'asc' + attr_reader :project, :user + # Returns a string describing what went wrong after # an operation fails. attr_reader :error_message - attr_reader :project def initialize(project, user = nil) @project = project @@ -196,9 +197,9 @@ class ProjectWiki def commit_details(action, message = nil, title = nil) commit_message = message.presence || default_message(action, title) - git_user = Gitlab::Git::User.from_gitlab(@user) + git_user = Gitlab::Git::User.from_gitlab(user) - Gitlab::Git::Wiki::CommitDetails.new(@user.id, + Gitlab::Git::Wiki::CommitDetails.new(user.id, git_user.username, git_user.name, git_user.email, @@ -206,7 +207,7 @@ class ProjectWiki end def default_message(action, title) - "#{@user.username} #{action} page: #{title}" + "#{user.username} #{action} page: #{title}" end def update_project_activity diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 5436f034657..fb65432024e 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -21,6 +21,14 @@ class WikiPage ActiveModel::Name.new(self, nil, 'wiki') end + def eql?(other) + return false unless other.present? && other.is_a?(self.class) + + slug == other.slug && wiki.project == other.wiki.project + end + + alias_method :==, :eql? + # Sorts and groups pages by directory. # # pages - an array of WikiPage objects. @@ -58,6 +66,7 @@ class WikiPage # The GitLab ProjectWiki instance. attr_reader :wiki + delegate :project, to: :wiki # The raw Gitlab::Git::WikiPage instance. attr_reader :page @@ -70,6 +79,10 @@ class WikiPage Gitlab::HookData::WikiPageBuilder.new(self).build end + # Construct a new WikiPage + # + # @param [ProjectWiki] wiki + # @param [Gitlab::Git::WikiPage] page def initialize(wiki, page = nil) @wiki = wiki @page = page diff --git a/app/models/wiki_page/meta.rb b/app/models/wiki_page/meta.rb new file mode 100644 index 00000000000..2af7d86ebcc --- /dev/null +++ b/app/models/wiki_page/meta.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +class WikiPage + class Meta < ApplicationRecord + include Gitlab::Utils::StrongMemoize + + CanonicalSlugConflictError = Class.new(ActiveRecord::RecordInvalid) + + self.table_name = 'wiki_page_meta' + + belongs_to :project + + has_many :slugs, class_name: 'WikiPage::Slug', foreign_key: 'wiki_page_meta_id', inverse_of: :wiki_page_meta + has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + + validates :title, presence: true + validates :project_id, presence: true + validate :no_two_metarecords_in_same_project_can_have_same_canonical_slug + + scope :with_canonical_slug, ->(slug) do + joins(:slugs).where(wiki_page_slugs: { canonical: true, slug: slug }) + end + + alias_method :resource_parent, :project + + # Return the (updated) WikiPage::Meta record for a given wiki page + # + # If none is found, then a new record is created, and its fields are set + # to reflect the wiki_page passed. + # + # @param [String] last_known_slug + # @param [WikiPage] wiki_page + # + # As with all `find_or_create` methods, this one raises errors on + # validation issues. + def self.find_or_create(last_known_slug, wiki_page) + project = wiki_page.wiki.project + known_slugs = [last_known_slug, wiki_page.slug].compact.uniq + raise 'no slugs!' if known_slugs.empty? + + transaction do + found = find_by_canonical_slug(known_slugs, project) + meta = found || create(title: wiki_page.title, project_id: project.id) + + meta.update_state(found.nil?, known_slugs, wiki_page) + + # We don't need to run validations here, since find_by_canonical_slug + # guarantees that there is no conflict in canonical_slug, and DB + # constraints on title and project_id enforce our other invariants + # This saves us a query. + meta + end + end + + def self.find_by_canonical_slug(canonical_slug, project) + meta, conflict = with_canonical_slug(canonical_slug) + .where(project_id: project.id) + .limit(2) + + if conflict.present? + meta.errors.add(:canonical_slug, 'Duplicate value found') + raise CanonicalSlugConflictError.new(meta) + end + + meta + end + + def canonical_slug + strong_memoize(:canonical_slug) { slugs.canonical.first&.slug } + end + + def canonical_slug=(slug) + return if @canonical_slug == slug + + if persisted? + transaction do + slugs.canonical.update_all(canonical: false) + page_slug = slugs.create_with(canonical: true).find_or_create_by(slug: slug) + page_slug.update_columns(canonical: true) unless page_slug.canonical? + end + else + slugs.new(slug: slug, canonical: true) + end + + @canonical_slug = slug + end + + def update_state(created, known_slugs, wiki_page) + update_wiki_page_attributes(wiki_page) + insert_slugs(known_slugs, created, wiki_page.slug) + self.canonical_slug = wiki_page.slug + end + + def update_columns(attrs = {}) + super(attrs.reverse_merge(updated_at: Time.now.utc)) + end + + def self.update_all(attrs = {}) + super(attrs.reverse_merge(updated_at: Time.now.utc)) + end + + private + + def update_wiki_page_attributes(page) + update_columns(title: page.title) unless page.title == title + end + + def insert_slugs(strings, is_new, canonical_slug) + creation = Time.now.utc + + slug_attrs = strings.map do |slug| + { + wiki_page_meta_id: id, + slug: slug, + canonical: (is_new && slug == canonical_slug), + created_at: creation, + updated_at: creation + } + end + slugs.insert_all(slug_attrs) unless !is_new && slug_attrs.size == 1 + + @canonical_slug = canonical_slug if is_new || strings.size == 1 + end + + def no_two_metarecords_in_same_project_can_have_same_canonical_slug + return unless project_id.present? && canonical_slug.present? + + offending = self.class.with_canonical_slug(canonical_slug).where(project_id: project_id) + offending = offending.where.not(id: id) if persisted? + + if offending.exists? + errors.add(:canonical_slug, 'each page in a wiki must have a distinct canonical slug') + end + end + end +end diff --git a/app/models/wiki_page/slug.rb b/app/models/wiki_page/slug.rb new file mode 100644 index 00000000000..246fa8d6442 --- /dev/null +++ b/app/models/wiki_page/slug.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class WikiPage + class Slug < ApplicationRecord + self.table_name = 'wiki_page_slugs' + + belongs_to :wiki_page_meta, class_name: 'WikiPage::Meta', inverse_of: :slugs + + validates :slug, presence: true, uniqueness: { scope: :wiki_page_meta_id } + validates :canonical, uniqueness: { + scope: :wiki_page_meta_id, + if: :canonical?, + message: 'Only one slug can be canonical per wiki metadata record' + } + + scope :canonical, -> { where(canonical: true) } + + def update_columns(attrs = {}) + super(attrs.reverse_merge(updated_at: Time.now.utc)) + end + + def self.update_all(attrs = {}) + super(attrs.reverse_merge(updated_at: Time.now.utc)) + end + end +end diff --git a/app/policies/wiki_page/meta_policy.rb b/app/policies/wiki_page/meta_policy.rb new file mode 100644 index 00000000000..1eab2d7ca48 --- /dev/null +++ b/app/policies/wiki_page/meta_policy.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class WikiPage::MetaPolicy < BasePolicy + delegate { @subject.project } +end diff --git a/app/services/metrics/dashboard/clone_dashboard_service.rb b/app/services/metrics/dashboard/clone_dashboard_service.rb index 3b06a7713d7..c31d6d326e3 100644 --- a/app/services/metrics/dashboard/clone_dashboard_service.rb +++ b/app/services/metrics/dashboard/clone_dashboard_service.rb @@ -16,7 +16,7 @@ module Metrics def sequences @sequences ||= { ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => [::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, - ::Gitlab::Metrics::Dashboard::Stages::ProjectMetricsInserter, + ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter, ::Gitlab::Metrics::Dashboard::Stages::Sorter].freeze }.freeze end diff --git a/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb b/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb index d705c3f3ce5..bcdc8a8844f 100644 --- a/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb +++ b/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb @@ -9,7 +9,7 @@ module Metrics DASHBOARD_NAME = 'Default' SEQUENCE = [ - STAGES::ProjectMetricsInserter, + STAGES::CustomMetricsInserter, STAGES::EndpointInserter, STAGES::Sorter ].freeze diff --git a/app/services/metrics/dashboard/system_dashboard_service.rb b/app/services/metrics/dashboard/system_dashboard_service.rb index aa8421e10d5..1c9d7f4968a 100644 --- a/app/services/metrics/dashboard/system_dashboard_service.rb +++ b/app/services/metrics/dashboard/system_dashboard_service.rb @@ -10,8 +10,8 @@ module Metrics SEQUENCE = [ STAGES::CommonMetricsInserter, - STAGES::ProjectMetricsInserter, - STAGES::ProjectMetricsDetailsInserter, + STAGES::CustomMetricsInserter, + STAGES::CustomMetricsDetailsInserter, STAGES::EndpointInserter, STAGES::Sorter ].freeze diff --git a/changelogs/unreleased/119208-limit-metric-type-on-list-pd.yml b/changelogs/unreleased/119208-limit-metric-type-on-list-pd.yml new file mode 100644 index 00000000000..2ac04a1b3dc --- /dev/null +++ b/changelogs/unreleased/119208-limit-metric-type-on-list-pd.yml @@ -0,0 +1,5 @@ +--- +title: Add limit metric to lists +merge_request: 25532 +author: +type: added diff --git a/changelogs/unreleased/207912-implementing-filtered-search-dropdown-improvement.yml b/changelogs/unreleased/207912-implementing-filtered-search-dropdown-improvement.yml new file mode 100644 index 00000000000..48abd425922 --- /dev/null +++ b/changelogs/unreleased/207912-implementing-filtered-search-dropdown-improvement.yml @@ -0,0 +1,5 @@ +--- +title: Improve logs dropdown with more clear labels +merge_request: 26635 +author: +type: added diff --git a/changelogs/unreleased/208234-optimize-ldap-keys-in-usage-data.yml b/changelogs/unreleased/208234-optimize-ldap-keys-in-usage-data.yml new file mode 100644 index 00000000000..2a0f7b13485 --- /dev/null +++ b/changelogs/unreleased/208234-optimize-ldap-keys-in-usage-data.yml @@ -0,0 +1,5 @@ +--- +title: Optimize ldap keys counters query performance in usage data +merge_request: 27309 +author: +type: performance diff --git a/changelogs/unreleased/30526-a-be-wiki-activity-Models.yml b/changelogs/unreleased/30526-a-be-wiki-activity-Models.yml new file mode 100644 index 00000000000..5e7e9396f65 --- /dev/null +++ b/changelogs/unreleased/30526-a-be-wiki-activity-Models.yml @@ -0,0 +1,5 @@ +--- +title: Adds wiki metadata models +merge_request: 26529 +author: +type: added diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 858766f19ca..08cb8f715f0 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -234,7 +234,7 @@ - 2 - - service_desk_email_receiver - 1 -- - status_page_publish_incident +- - status_page_publish - 1 - - sync_seat_link_request - 1 diff --git a/db/migrate/20200219135440_add_limit_metric_type_to_list.rb b/db/migrate/20200219135440_add_limit_metric_type_to_list.rb new file mode 100644 index 00000000000..35e36b4805b --- /dev/null +++ b/db/migrate/20200219135440_add_limit_metric_type_to_list.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddLimitMetricTypeToList < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column :lists, :limit_metric, :string, limit: 20 + end +end diff --git a/db/migrate/20200302152516_add_wiki_slug.rb b/db/migrate/20200302152516_add_wiki_slug.rb new file mode 100644 index 00000000000..dabac39aeb8 --- /dev/null +++ b/db/migrate/20200302152516_add_wiki_slug.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddWikiSlug < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + create_table :wiki_page_meta, id: :serial do |t| + t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false + t.timestamps_with_timezone null: false + t.string :title, null: false, limit: 255 + end + + create_table :wiki_page_slugs, id: :serial do |t| + t.boolean :canonical, default: false, null: false + t.references :wiki_page_meta, index: true, foreign_key: { on_delete: :cascade }, null: false + t.timestamps_with_timezone null: false + t.string :slug, null: false, limit: 2048 + t.index [:slug, :wiki_page_meta_id], unique: true + t.index [:wiki_page_meta_id], name: 'one_canonical_wiki_page_slug_per_metadata', unique: true, where: "(canonical = true)" + end + end +end diff --git a/db/migrate/20200316111759_add_index_on_id_and_ldap_key_to_keys.rb b/db/migrate/20200316111759_add_index_on_id_and_ldap_key_to_keys.rb new file mode 100644 index 00000000000..23e7b5ba7b9 --- /dev/null +++ b/db/migrate/20200316111759_add_index_on_id_and_ldap_key_to_keys.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddIndexOnIdAndLdapKeyToKeys < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_keys_on_id_and_ldap_key_type' + + disable_ddl_transaction! + + def up + add_concurrent_index :keys, [:id], where: "type = 'LDAPKey'", name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :keys, INDEX_NAME + end +end diff --git a/db/schema.rb b/db/schema.rb index aed7285e698..69ef887bfe5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_03_13_123934) do +ActiveRecord::Schema.define(version: 2020_03_16_111759) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -2311,6 +2311,7 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do t.index ["fingerprint"], name: "index_keys_on_fingerprint", unique: true t.index ["fingerprint_sha256"], name: "index_keys_on_fingerprint_sha256" t.index ["id", "type"], name: "index_on_deploy_keys_id_and_type_and_public", unique: true, where: "(public = true)" + t.index ["id"], name: "index_keys_on_id_and_ldap_key_type", where: "((type)::text = 'LDAPKey'::text)" t.index ["last_used_at"], name: "index_keys_on_last_used_at", order: "DESC NULLS LAST" t.index ["user_id"], name: "index_keys_on_user_id" end @@ -2425,6 +2426,7 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do t.integer "milestone_id" t.integer "max_issue_count", default: 0, null: false t.integer "max_issue_weight", default: 0, null: false + t.string "limit_metric", limit: 20 t.index ["board_id", "label_id"], name: "index_lists_on_board_id_and_label_id", unique: true t.index ["label_id"], name: "index_lists_on_label_id" t.index ["list_type"], name: "index_lists_on_list_type" @@ -4668,6 +4670,25 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do t.index ["type"], name: "index_web_hooks_on_type" end + create_table "wiki_page_meta", id: :serial, force: :cascade do |t| + t.bigint "project_id", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.string "title", limit: 255, null: false + t.index ["project_id"], name: "index_wiki_page_meta_on_project_id" + end + + create_table "wiki_page_slugs", id: :serial, force: :cascade do |t| + t.boolean "canonical", default: false, null: false + t.bigint "wiki_page_meta_id", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.string "slug", limit: 2048, null: false + t.index ["slug", "wiki_page_meta_id"], name: "index_wiki_page_slugs_on_slug_and_wiki_page_meta_id", unique: true + t.index ["wiki_page_meta_id"], name: "index_wiki_page_slugs_on_wiki_page_meta_id" + t.index ["wiki_page_meta_id"], name: "one_canonical_wiki_page_slug_per_metadata", unique: true, where: "(canonical = true)" + end + create_table "x509_certificates", force: :cascade do |t| t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false @@ -5212,6 +5233,8 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do add_foreign_key "vulnerability_scanners", "projects", on_delete: :cascade add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade add_foreign_key "web_hooks", "projects", name: "fk_0c8ca6d9d1", on_delete: :cascade + add_foreign_key "wiki_page_meta", "projects", on_delete: :cascade + add_foreign_key "wiki_page_slugs", "wiki_page_meta", column: "wiki_page_meta_id", on_delete: :cascade add_foreign_key "x509_certificates", "x509_issuers", on_delete: :cascade add_foreign_key "x509_commit_signatures", "projects", on_delete: :cascade add_foreign_key "x509_commit_signatures", "x509_certificates", on_delete: :cascade diff --git a/doc/development/architecture.md b/doc/development/architecture.md index c75de8e8970..d1ae4fbec18 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -494,7 +494,9 @@ Below we describe the different pathing that HTTP vs. SSH Git requests will take ### Web Request (80/443) -TODO +When you make a Git request over HTTP, the request first takes the same steps as a web HTTP request +through NGINX and GitLab Workhorse. However, the GitLab Workhorse then diverts the request towards +Gitaly, which processes it directly. ### SSH Request (22) diff --git a/doc/development/changelog.md b/doc/development/changelog.md index 8db3346d1f1..bc1ba4eb50b 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -44,8 +44,9 @@ the `author` field. GitLab team members **should not**. a changelog entry regardless of these guidelines if the contributor wants one. Example: "Fixed a typo on the search results page." - Any docs-only changes **should not** have a changelog entry. -- Any change behind a feature flag **should not** have a changelog entry. The - entry should be added [in the merge request removing the feature flags](feature_flags/development.md). +- Any change behind a feature flag **should not** have a changelog entry - unless + the feature flag has been defaulted to true. The entry should be added + [in the merge request removing the feature flags](feature_flags/development.md). If the change includes a database migration (regular, post, or data migration), there should be a changelog entry for the migration change. - A fix for a regression introduced and then fixed in the same release (i.e., diff --git a/lib/gitlab/hook_data/wiki_page_builder.rb b/lib/gitlab/hook_data/wiki_page_builder.rb index 67f06b1ca46..78ae896a3d8 100644 --- a/lib/gitlab/hook_data/wiki_page_builder.rb +++ b/lib/gitlab/hook_data/wiki_page_builder.rb @@ -12,6 +12,10 @@ module Gitlab 'content' => absolute_image_urls(wiki_page.content) ) end + + def uploads_prefix + '' + end end end end diff --git a/lib/gitlab/metrics/dashboard/stages/custom_metrics_details_inserter.rb b/lib/gitlab/metrics/dashboard/stages/custom_metrics_details_inserter.rb new file mode 100644 index 00000000000..cfec027155e --- /dev/null +++ b/lib/gitlab/metrics/dashboard/stages/custom_metrics_details_inserter.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + module Stages + class CustomMetricsDetailsInserter < BaseStage + def transform! + dashboard[:panel_groups].each do |panel_group| + next unless panel_group + + has_custom_metrics = custom_group_titles.include?(panel_group[:group]) + panel_group[:has_custom_metrics] = has_custom_metrics + + panel_group[:panels].each do |panel| + next unless panel + + panel[:metrics].each do |metric| + next unless metric + + metric[:edit_path] = has_custom_metrics ? edit_path(metric) : nil + end + end + end + end + + private + + def custom_group_titles + @custom_group_titles ||= PrometheusMetricEnums.custom_group_details.values.map { |group_details| group_details[:group_title] } + end + + def edit_path(metric) + Gitlab::Routing.url_helpers.edit_project_prometheus_metric_path(project, metric[:metric_id]) + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/stages/custom_metrics_inserter.rb b/lib/gitlab/metrics/dashboard/stages/custom_metrics_inserter.rb new file mode 100644 index 00000000000..3444a01bccd --- /dev/null +++ b/lib/gitlab/metrics/dashboard/stages/custom_metrics_inserter.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + module Stages + class CustomMetricsInserter < BaseStage + # Inserts project-specific metrics into the dashboard + # config. If there are no project-specific metrics, + # this will have no effect. + def transform! + PrometheusMetricsFinder.new(project: project).execute.each do |project_metric| + group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) + panel = find_or_create_panel(group[:panels], project_metric) + find_or_create_metric(panel[:metrics], project_metric) + end + end + + private + + # Looks for a panel_group corresponding to the + # provided metric object. If unavailable, inserts one. + # @param panel_groups [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel_group(panel_groups, metric) + panel_group = find_panel_group(panel_groups, metric) + return panel_group if panel_group + + panel_group = new_panel_group(metric) + panel_groups << panel_group + + panel_group + end + + # Looks for a panel corresponding to the provided + # metric object. If unavailable, inserts one. + # @param panels [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel(panels, metric) + panel = find_panel(panels, metric) + return panel if panel + + panel = new_panel(metric) + panels << panel + + panel + end + + # Looks for a metric corresponding to the provided + # metric object. If unavailable, inserts one. + # @param metrics [Array] + # @param metric [PrometheusMetric] + def find_or_create_metric(metrics, metric) + target_metric = find_metric(metrics, metric) + return target_metric if target_metric + + target_metric = new_metric(metric) + metrics << target_metric + + target_metric + end + + def find_panel_group(panel_groups, metric) + return unless panel_groups + + panel_groups.find { |group| group[:group] == metric.group_title } + end + + def find_panel(panels, metric) + return unless panels + + panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] + panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } + end + + def find_metric(metrics, metric) + return unless metrics + return unless metric.identifier + + metrics.find { |m| m[:id] == metric.identifier } + end + + def new_panel_group(metric) + { + group: metric.group_title, + priority: metric.priority, + panels: [] + } + end + + def new_panel(metric) + { + type: DEFAULT_PANEL_TYPE, + title: metric.title, + y_label: metric.y_label, + metrics: [] + } + end + + def new_metric(metric) + metric.to_metric_hash + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/stages/project_metrics_details_inserter.rb b/lib/gitlab/metrics/dashboard/stages/project_metrics_details_inserter.rb deleted file mode 100644 index a9d11f58255..00000000000 --- a/lib/gitlab/metrics/dashboard/stages/project_metrics_details_inserter.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Metrics - module Dashboard - module Stages - class ProjectMetricsDetailsInserter < BaseStage - def transform! - dashboard[:panel_groups].each do |panel_group| - next unless panel_group - - has_custom_metrics = custom_group_titles.include?(panel_group[:group]) - panel_group[:has_custom_metrics] = has_custom_metrics - - panel_group[:panels].each do |panel| - next unless panel - - panel[:metrics].each do |metric| - next unless metric - - metric[:edit_path] = has_custom_metrics ? edit_path(metric) : nil - end - end - end - end - - private - - def custom_group_titles - @custom_group_titles ||= PrometheusMetricEnums.custom_group_details.values.map { |group_details| group_details[:group_title] } - end - - def edit_path(metric) - Gitlab::Routing.url_helpers.edit_project_prometheus_metric_path(project, metric[:metric_id]) - end - end - end - end - end -end diff --git a/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb deleted file mode 100644 index 26345c265ef..00000000000 --- a/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb +++ /dev/null @@ -1,107 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Metrics - module Dashboard - module Stages - class ProjectMetricsInserter < BaseStage - # Inserts project-specific metrics into the dashboard - # config. If there are no project-specific metrics, - # this will have no effect. - def transform! - PrometheusMetricsFinder.new(project: project).execute.each do |project_metric| - group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) - panel = find_or_create_panel(group[:panels], project_metric) - find_or_create_metric(panel[:metrics], project_metric) - end - end - - private - - # Looks for a panel_group corresponding to the - # provided metric object. If unavailable, inserts one. - # @param panel_groups [Array] - # @param metric [PrometheusMetric] - def find_or_create_panel_group(panel_groups, metric) - panel_group = find_panel_group(panel_groups, metric) - return panel_group if panel_group - - panel_group = new_panel_group(metric) - panel_groups << panel_group - - panel_group - end - - # Looks for a panel corresponding to the provided - # metric object. If unavailable, inserts one. - # @param panels [Array] - # @param metric [PrometheusMetric] - def find_or_create_panel(panels, metric) - panel = find_panel(panels, metric) - return panel if panel - - panel = new_panel(metric) - panels << panel - - panel - end - - # Looks for a metric corresponding to the provided - # metric object. If unavailable, inserts one. - # @param metrics [Array] - # @param metric [PrometheusMetric] - def find_or_create_metric(metrics, metric) - target_metric = find_metric(metrics, metric) - return target_metric if target_metric - - target_metric = new_metric(metric) - metrics << target_metric - - target_metric - end - - def find_panel_group(panel_groups, metric) - return unless panel_groups - - panel_groups.find { |group| group[:group] == metric.group_title } - end - - def find_panel(panels, metric) - return unless panels - - panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] - panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } - end - - def find_metric(metrics, metric) - return unless metrics - return unless metric.identifier - - metrics.find { |m| m[:id] == metric.identifier } - end - - def new_panel_group(metric) - { - group: metric.group_title, - priority: metric.priority, - panels: [] - } - end - - def new_panel(metric) - { - type: DEFAULT_PANEL_TYPE, - title: metric.title, - y_label: metric.y_label, - metrics: [] - } - end - - def new_metric(metric) - metric.to_metric_hash - end - end - end - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9893d50ce4c..8c0d638c617 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7723,6 +7723,9 @@ msgstr "" msgid "Environments|Environments are places where code gets deployed, such as staging or production." msgstr "" +msgid "Environments|Filter by pod" +msgstr "" + msgid "Environments|Install Elastic Stack on your cluster to enable advanced querying capabilities such as full text search." msgstr "" @@ -7735,9 +7738,6 @@ msgstr "" msgid "Environments|Learn more about stopping environments" msgstr "" -msgid "Environments|Logs from" -msgstr "" - msgid "Environments|Logs from %{start} to %{end}." msgstr "" @@ -7753,6 +7753,9 @@ msgstr "" msgid "Environments|No pod selected" msgstr "" +msgid "Environments|No pods to display" +msgstr "" + msgid "Environments|Note that this action will stop the environment, but it will %{emphasisStart}not%{emphasisEnd} have an effect on any existing deployment due to no “stop environment action” being defined in the %{ciConfigLinkStart}.gitlab-ci.yml%{ciConfigLinkEnd} file." msgstr "" @@ -7792,10 +7795,10 @@ msgstr "" msgid "Environments|Search" msgstr "" -msgid "Environments|Show all" +msgid "Environments|Select environment" msgstr "" -msgid "Environments|Show last" +msgid "Environments|Show all" msgstr "" msgid "Environments|Stop" diff --git a/spec/factories/events.rb b/spec/factories/events.rb index 81d57a25058..b4285627de3 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -22,6 +22,16 @@ FactoryBot.define do action { Event::CLOSED } target factory: :closed_issue end + + factory :wiki_page_event do + action { Event::CREATED } + + transient do + wiki_page { create(:wiki_page, project: project) } + end + + target { create(:wiki_page_meta, :for_wiki_page, wiki_page: wiki_page) } + end end factory :push_event, class: 'PushEvent' do diff --git a/spec/factories/lists.rb b/spec/factories/lists.rb index cf00351b231..623b080c073 100644 --- a/spec/factories/lists.rb +++ b/spec/factories/lists.rb @@ -7,6 +7,7 @@ FactoryBot.define do list_type { :label } max_issue_count { 0 } max_issue_weight { 0 } + limit_metric { nil } sequence(:position) end diff --git a/spec/factories/wiki_pages.rb b/spec/factories/wiki_pages.rb index 86571f062ba..8eb7a12a928 100644 --- a/spec/factories/wiki_pages.rb +++ b/spec/factories/wiki_pages.rb @@ -5,17 +5,22 @@ require 'ostruct' FactoryBot.define do factory :wiki_page do transient do + title { generate(:wiki_page_title) } + content { 'Content for wiki page' } + format { 'markdown' } + project { create(:project) } attrs do { - title: 'Title.with.dot', - content: 'Content for wiki page', - format: 'markdown' + title: title, + content: content, + format: format } end end page { OpenStruct.new(url_path: 'some-name') } - association :wiki, factory: :project_wiki, strategy: :build + wiki { build(:project_wiki, project: project) } + initialize_with { new(wiki, page) } before(:create) do |page, evaluator| @@ -25,5 +30,48 @@ FactoryBot.define do to_create do |page| page.create end + + trait :with_real_page do + project { create(:project, :repository) } + + page do + wiki.create_page(title, content) + page_title, page_dir = wiki.page_title_and_dir(title) + wiki.wiki.page(title: page_title, dir: page_dir, version: nil) + end + end end + + factory :wiki_page_meta, class: 'WikiPage::Meta' do + title { generate(:wiki_page_title) } + project { create(:project) } + + trait :for_wiki_page do + transient do + wiki_page { create(:wiki_page, project: project) } + end + + project { @overrides[:wiki_page]&.project || create(:project) } + title { wiki_page.title } + + initialize_with do + raise 'Metadata only available for valid pages' unless wiki_page.valid? + + WikiPage::Meta.find_or_create(wiki_page.slug, wiki_page) + end + end + end + + factory :wiki_page_slug, class: 'WikiPage::Slug' do + wiki_page_meta { create(:wiki_page_meta) } + slug { generate(:sluggified_title) } + canonical { false } + + trait :canonical do + canonical { true } + end + end + + sequence(:wiki_page_title) { |n| "Page #{n}" } + sequence(:sluggified_title) { |n| "slug-#{n}" } end diff --git a/spec/fixtures/api/schemas/list.json b/spec/fixtures/api/schemas/list.json index 760dcb96252..f894ff584c8 100644 --- a/spec/fixtures/api/schemas/list.json +++ b/spec/fixtures/api/schemas/list.json @@ -37,7 +37,8 @@ "title": { "type": "string" }, "position": { "type": ["integer", "null"] }, "max_issue_count": { "type": "integer" }, - "max_issue_weight": { "type": "integer" } + "max_issue_weight": { "type": "integer" }, + "limit_metric": { "type": ["string", "null"] } }, "additionalProperties": true } diff --git a/spec/frontend/logs/components/environment_logs_spec.js b/spec/frontend/logs/components/environment_logs_spec.js index 162aeb1cc56..49642153c69 100644 --- a/spec/frontend/logs/components/environment_logs_spec.js +++ b/spec/frontend/logs/components/environment_logs_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import { GlSprintf, GlDropdown, GlDropdownItem, GlSearchBoxByClick } from '@gitlab/ui'; +import { GlSprintf, GlIcon, GlDropdown, GlDropdownItem, GlSearchBoxByClick } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import DateTimePicker from '~/vue_shared/components/date_time_picker/date_time_picker.vue'; import EnvironmentLogs from '~/logs/components/environment_logs.vue'; @@ -45,6 +45,10 @@ describe('EnvironmentLogs', () => { const findEnvironmentsDropdown = () => wrapper.find('.js-environments-dropdown'); const findPodsDropdown = () => wrapper.find('.js-pods-dropdown'); + const findPodsDropdownItems = () => + findPodsDropdown() + .findAll(GlDropdownItem) + .filter(itm => !itm.attributes('disabled')); const findSearchBar = () => wrapper.find('.js-logs-search'); const findTimeRangePicker = () => wrapper.find({ ref: 'dateTimePicker' }); const findInfoAlert = () => wrapper.find('.js-elasticsearch-alert'); @@ -179,7 +183,7 @@ describe('EnvironmentLogs', () => { it('displays a disabled pods dropdown', () => { expect(findPodsDropdown().attributes('disabled')).toBe('true'); - expect(findPodsDropdown().findAll(GlDropdownItem).length).toBe(0); + expect(findPodsDropdownItems()).toHaveLength(0); }); it('displays a disabled search bar', () => { @@ -296,8 +300,22 @@ describe('EnvironmentLogs', () => { }); }); + it('dropdown has one environment selected', () => { + const items = findEnvironmentsDropdown().findAll(GlDropdownItem); + mockEnvironments.forEach((env, i) => { + const item = items.at(i); + + if (item.text() !== mockEnvName) { + expect(item.find(GlIcon).classes()).toContain('invisible'); + } else { + // selected + expect(item.find(GlIcon).classes()).not.toContain('invisible'); + } + }); + }); + it('populates pods dropdown', () => { - const items = findPodsDropdown().findAll(GlDropdownItem); + const items = findPodsDropdownItems(); expect(findPodsDropdown().props('text')).toBe(mockPodName); expect(items.length).toBe(mockPods.length + 1); @@ -313,6 +331,19 @@ describe('EnvironmentLogs', () => { expect(getInfiniteScrollAttr('fetched-items')).toBe(mockTrace.length); }); + it('dropdown has one pod selected', () => { + const items = findPodsDropdownItems(); + mockPods.forEach((pod, i) => { + const item = items.at(i); + if (item.text() !== mockPodName) { + expect(item.find(GlIcon).classes()).toContain('invisible'); + } else { + // selected + expect(item.find(GlIcon).classes()).not.toContain('invisible'); + } + }); + }); + it('populates logs trace', () => { const trace = findLogTrace(); expect(trace.text().split('\n').length).toBe(mockTrace.length); @@ -341,7 +372,7 @@ describe('EnvironmentLogs', () => { }); it('pod name, trace is refreshed', () => { - const items = findPodsDropdown().findAll(GlDropdownItem); + const items = findPodsDropdownItems(); const index = 2; // any pod expect(dispatch).not.toHaveBeenCalledWith(`${module}/showPodLogs`, expect.anything()); diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb index 2547d39bcb2..9251eaee3df 100644 --- a/spec/graphql/types/base_field_spec.rb +++ b/spec/graphql/types/base_field_spec.rb @@ -167,30 +167,30 @@ describe Types::BaseField do end end end + end - describe '#description' do - context 'feature flag given' do - let(:field) { described_class.new(name: 'test', type: GraphQL::STRING_TYPE, feature_flag: flag, null: false, description: 'Test description') } - let(:flag) { :test_flag } + describe '#description' do + context 'feature flag given' do + let(:field) { described_class.new(name: 'test', type: GraphQL::STRING_TYPE, feature_flag: flag, null: false, description: 'Test description') } + let(:flag) { :test_flag } - it 'prepends the description' do - expect(field.description). to eq 'Test description. Available only when feature flag `test_flag` is enabled.' - end + it 'prepends the description' do + expect(field.description). to eq 'Test description. Available only when feature flag `test_flag` is enabled.' + end - context 'falsey feature_flag values' do - using RSpec::Parameterized::TableSyntax + context 'falsey feature_flag values' do + using RSpec::Parameterized::TableSyntax - where(:flag, :feature_value) do - '' | false - '' | true - nil | false - nil | true - end + where(:flag, :feature_value) do + '' | false + '' | true + nil | false + nil | true + end - with_them do - it 'returns the correct description' do - expect(field.description).to eq('Test description') - end + with_them do + it 'returns the correct description' do + expect(field.description).to eq('Test description') end end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 91b88349ee0..4c9caaa181a 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -40,6 +40,14 @@ Event: - updated_at - action - author_id +WikiPage::Meta: +- id +- title +- project_id +WikiPage::Slug: +- id +- wiki_page_meta_id +- slug PushEventPayload: - commit_count - action @@ -756,6 +764,7 @@ List: - user_id - max_issue_count - max_issue_weight +- limit_metric ExternalPullRequest: - id - created_at diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index 41693a991e0..d957b1c992f 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -11,8 +11,8 @@ describe Gitlab::Metrics::Dashboard::Processor do let(:sequence) do [ Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, - Gitlab::Metrics::Dashboard::Stages::ProjectMetricsInserter, - Gitlab::Metrics::Dashboard::Stages::ProjectMetricsDetailsInserter, + Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter, + Gitlab::Metrics::Dashboard::Stages::CustomMetricsDetailsInserter, Gitlab::Metrics::Dashboard::Stages::EndpointInserter, Gitlab::Metrics::Dashboard::Stages::Sorter ] diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 97ea32a120d..b2676a79b55 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -99,6 +99,39 @@ describe Event do end end + describe '#target_title' do + let_it_be(:project) { create(:project) } + + let(:author) { project.owner } + let(:target) { nil } + + let(:event) do + described_class.new(project: project, + target: target, + author_id: author.id) + end + + context 'for an issue' do + let(:title) { generate(:title) } + let(:issue) { create(:issue, title: title, project: project) } + let(:target) { issue } + + it 'delegates to issue title' do + expect(event.target_title).to eq(title) + end + end + + context 'for a wiki page' do + let(:title) { generate(:wiki_page_title) } + let(:wiki_page) { create(:wiki_page, title: title, project: project) } + let(:event) { create(:wiki_page_event, project: project, wiki_page: wiki_page) } + + it 'delegates to wiki page title' do + expect(event.target_title).to eq(wiki_page.title) + end + end + end + describe '#membership_changed?' do context "created" do subject { build(:event, :created).membership_changed? } @@ -148,13 +181,16 @@ describe Event do end describe '#visible_to_user?' do - let(:project) { create(:project, :public) } - let(:non_member) { create(:user) } - let(:member) { create(:user) } - let(:guest) { create(:user) } - let(:author) { create(:author) } - let(:assignee) { create(:user) } - let(:admin) { create(:admin) } + let_it_be(:non_member) { create(:user) } + let_it_be(:member) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:author) { create(:author) } + let_it_be(:assignee) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:private_project) { create(:project, :private) } + + let(:project) { public_project } let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:project_snippet) { create(:project_snippet, :public, project: project, author: author) } @@ -165,36 +201,77 @@ describe Event do let(:note_on_project_snippet) { create(:note_on_project_snippet, author: author, noteable: project_snippet, project: project) } let(:note_on_personal_snippet) { create(:note_on_personal_snippet, author: author, noteable: personal_snippet, project: nil) } let(:milestone_on_project) { create(:milestone, project: project) } - let(:event) { described_class.new(project: project, target: target, author_id: author.id) } + let(:event) do + described_class.new(project: project, + target: target, + author_id: author.id) + end before do project.add_developer(member) project.add_guest(guest) end + def visible_to_all + { + logged_out: true, + non_member: true, + guest: true, + member: true, + admin: true + } + end + + def visible_to_none + visible_to_all.transform_values { |_| false } + end + + def visible_to_none_except(*roles) + visible_to_none.merge(roles.map { |role| [role, true] }.to_h) + end + + def visible_to_all_except(*roles) + visible_to_all.merge(roles.map { |role| [role, false] }.to_h) + end + + shared_examples 'visibility examples' do + it 'has the correct visibility' do + expect({ + logged_out: event.visible_to_user?(nil), + non_member: event.visible_to_user?(non_member), + guest: event.visible_to_user?(guest), + member: event.visible_to_user?(member), + admin: event.visible_to_user?(admin) + }).to match(visibility) + end + end + + shared_examples 'visible to assignee' do |visible| + it { expect(event.visible_to_user?(assignee)).to eq(visible) } + end + + shared_examples 'visible to author' do |visible| + it { expect(event.visible_to_user?(author)).to eq(visible) } + end + + shared_examples 'visible to assignee and author' do |visible| + include_examples 'visible to assignee', visible + include_examples 'visible to author', visible + end + context 'commit note event' do let(:project) { create(:project, :public, :repository) } let(:target) { note_on_commit } - it do - aggregate_failures do - expect(event.visible_to_user?(non_member)).to eq true - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq true - expect(event.visible_to_user?(admin)).to eq true - end + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } end context 'private project' do let(:project) { create(:project, :private, :repository) } - it do - aggregate_failures do - expect(event.visible_to_user?(non_member)).to eq false - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq false - expect(event.visible_to_user?(admin)).to eq true - end + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:member, :admin) } end end end @@ -203,27 +280,19 @@ describe Event do context 'for non confidential issues' do let(:target) { issue } - it do - expect(event.visible_to_user?(non_member)).to eq true - expect(event.visible_to_user?(author)).to eq true - expect(event.visible_to_user?(assignee)).to eq true - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq true - expect(event.visible_to_user?(admin)).to eq true + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } end + include_examples 'visible to assignee and author', true end context 'for confidential issues' do let(:target) { confidential_issue } - it do - expect(event.visible_to_user?(non_member)).to eq false - expect(event.visible_to_user?(author)).to eq true - expect(event.visible_to_user?(assignee)).to eq true - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq false - expect(event.visible_to_user?(admin)).to eq true + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:member, :admin) } end + include_examples 'visible to assignee and author', true end end @@ -231,105 +300,99 @@ describe Event do context 'on non confidential issues' do let(:target) { note_on_issue } - it do - expect(event.visible_to_user?(non_member)).to eq true - expect(event.visible_to_user?(author)).to eq true - expect(event.visible_to_user?(assignee)).to eq true - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq true - expect(event.visible_to_user?(admin)).to eq true + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } end + include_examples 'visible to assignee and author', true end context 'on confidential issues' do let(:target) { note_on_confidential_issue } - it do - expect(event.visible_to_user?(non_member)).to eq false - expect(event.visible_to_user?(author)).to eq true - expect(event.visible_to_user?(assignee)).to eq true - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq false - expect(event.visible_to_user?(admin)).to eq true + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:member, :admin) } end + include_examples 'visible to assignee and author', true end context 'private project' do - let(:project) { create(:project, :private) } + let(:project) { private_project } let(:target) { note_on_issue } - it do - expect(event.visible_to_user?(non_member)).to eq false - expect(event.visible_to_user?(author)).to eq false - expect(event.visible_to_user?(assignee)).to eq false - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq true - expect(event.visible_to_user?(admin)).to eq true + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:guest, :member, :admin) } end + + include_examples 'visible to assignee and author', false end end context 'merge request diff note event' do - let(:project) { create(:project, :public) } let(:merge_request) { create(:merge_request, source_project: project, author: author, assignees: [assignee]) } let(:note_on_merge_request) { create(:legacy_diff_note_on_merge_request, noteable: merge_request, project: project) } let(:target) { note_on_merge_request } - it do - expect(event.visible_to_user?(non_member)).to eq true - expect(event.visible_to_user?(author)).to eq true - expect(event.visible_to_user?(assignee)).to eq true - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq true - expect(event.visible_to_user?(admin)).to eq true + context 'public project' do + let(:project) { public_project } + + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } + end + + include_examples 'visible to assignee', true end context 'private project' do - let(:project) { create(:project, :private) } + let(:project) { private_project } - it do - expect(event.visible_to_user?(non_member)).to eq false - expect(event.visible_to_user?(author)).to eq false - expect(event.visible_to_user?(assignee)).to eq false - expect(event.visible_to_user?(member)).to eq true - expect(event.visible_to_user?(guest)).to eq false - expect(event.visible_to_user?(admin)).to eq true + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:member, :admin) } end + + include_examples 'visible to assignee', false end end context 'milestone event' do let(:target) { milestone_on_project } - it do - expect(event.visible_to_user?(nil)).to be_truthy - expect(event.visible_to_user?(non_member)).to be_truthy - expect(event.visible_to_user?(member)).to be_truthy - expect(event.visible_to_user?(guest)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } end context 'on public project with private issue tracker and merge requests' do let(:project) { create(:project, :public, :issues_private, :merge_requests_private) } - it do - expect(event.visible_to_user?(nil)).to be_falsy - expect(event.visible_to_user?(non_member)).to be_falsy - expect(event.visible_to_user?(member)).to be_truthy - expect(event.visible_to_user?(guest)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_all_except(:logged_out, :non_member) } end end context 'on private project' do let(:project) { create(:project, :private) } - it do - expect(event.visible_to_user?(nil)).to be_falsy - expect(event.visible_to_user?(non_member)).to be_falsy - expect(event.visible_to_user?(member)).to be_truthy - expect(event.visible_to_user?(guest)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_all_except(:logged_out, :non_member) } + end + end + end + + context 'wiki-page event', :aggregate_failures do + let(:event) { create(:wiki_page_event, project: project) } + + context 'on private project', :aggregate_failures do + let(:project) { create(:project, :wiki_repo) } + + include_examples 'visibility examples' do + let(:visibility) { visible_to_all_except(:logged_out, :non_member) } + end + end + + context 'wiki-page event on public project', :aggregate_failures do + let(:project) { create(:project, :public, :wiki_repo) } + + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } end end end @@ -337,79 +400,98 @@ describe Event do context 'project snippet note event' do let(:target) { note_on_project_snippet } - it do - expect(event.visible_to_user?(nil)).to be_truthy - expect(event.visible_to_user?(non_member)).to be_truthy - expect(event.visible_to_user?(author)).to be_truthy - expect(event.visible_to_user?(member)).to be_truthy - expect(event.visible_to_user?(guest)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } end context 'on public project with private snippets' do let(:project) { create(:project, :public, :snippets_private) } - it do - expect(event.visible_to_user?(nil)).to be_falsy - expect(event.visible_to_user?(non_member)).to be_falsy - - # Normally, we'd expect the author of a comment to be able to view it. - # However, this doesn't seem to be the case for comments on snippets. - expect(event.visible_to_user?(author)).to be_falsy - - expect(event.visible_to_user?(member)).to be_truthy - expect(event.visible_to_user?(guest)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:guest, :member, :admin) } end + # Normally, we'd expect the author of a comment to be able to view it. + # However, this doesn't seem to be the case for comments on snippets. + include_examples 'visible to author', false end context 'on private project' do let(:project) { create(:project, :private) } - it do - expect(event.visible_to_user?(nil)).to be_falsy - expect(event.visible_to_user?(non_member)).to be_falsy - - # Normally, we'd expect the author of a comment to be able to view it. - # However, this doesn't seem to be the case for comments on snippets. - expect(event.visible_to_user?(author)).to be_falsy - - expect(event.visible_to_user?(member)).to be_truthy - expect(event.visible_to_user?(guest)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:guest, :member, :admin) } end + # Normally, we'd expect the author of a comment to be able to view it. + # However, this doesn't seem to be the case for comments on snippets. + include_examples 'visible to author', false end end context 'personal snippet note event' do let(:target) { note_on_personal_snippet } - it do - expect(event.visible_to_user?(nil)).to be_truthy - expect(event.visible_to_user?(non_member)).to be_truthy - expect(event.visible_to_user?(author)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } end + include_examples 'visible to author', true context 'on internal snippet' do let(:personal_snippet) { create(:personal_snippet, :internal, author: author) } - it do - expect(event.visible_to_user?(nil)).to be_falsy - expect(event.visible_to_user?(non_member)).to be_truthy - expect(event.visible_to_user?(author)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_all_except(:logged_out) } end end context 'on private snippet' do let(:personal_snippet) { create(:personal_snippet, :private, author: author) } - it do - expect(event.visible_to_user?(nil)).to be_falsy - expect(event.visible_to_user?(non_member)).to be_falsy - expect(event.visible_to_user?(author)).to be_truthy - expect(event.visible_to_user?(admin)).to be_truthy + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:admin) } + end + include_examples 'visible to author', true + end + end + end + + describe '.for_wiki_page' do + let_it_be(:events) do + [ + create(:closed_issue_event), + create(:wiki_page_event), + create(:closed_issue_event), + create(:event, :created), + create(:wiki_page_event) + ] + end + + it 'only contains the wiki page events' do + wiki_events = events.select(&:wiki_page?) + + expect(described_class.for_wiki_page).to match_array(wiki_events) + end + end + + describe '#wiki_page and #wiki_page?' do + let_it_be(:project) { create(:project, :repository) } + + context 'for a wiki page event' do + let(:wiki_page) do + create(:wiki_page, :with_real_page, project: project) + end + + subject(:event) { create(:wiki_page_event, project: project, wiki_page: wiki_page) } + + it { is_expected.to have_attributes(wiki_page?: be_truthy, wiki_page: wiki_page) } + end + + [:issue, :user, :merge_request, :snippet, :milestone, nil].each do |kind| + context "for a #{kind} event" do + it 'is nil' do + target = create(kind) if kind + event = create(:event, project: project, target: target) + + expect(event).to have_attributes(wiki_page: be_nil, wiki_page?: be_falsy) end end end diff --git a/spec/models/wiki_page/meta_spec.rb b/spec/models/wiki_page/meta_spec.rb new file mode 100644 index 00000000000..f9bfc31ba64 --- /dev/null +++ b/spec/models/wiki_page/meta_spec.rb @@ -0,0 +1,430 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe WikiPage::Meta do + let_it_be(:project) { create(:project) } + let_it_be(:other_project) { create(:project) } + + describe 'Associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_many(:slugs) } + it { is_expected.to have_many(:events) } + + it 'can find slugs' do + meta = create(:wiki_page_meta) + slugs = create_list(:wiki_page_slug, 3, wiki_page_meta: meta) + + expect(meta.slugs).to match_array(slugs) + end + end + + describe 'Validations' do + subject do + described_class.new(title: 'some title', project: project) + end + + it { is_expected.to validate_presence_of(:project_id) } + it { is_expected.to validate_presence_of(:title) } + + it 'is forbidden to add extremely long titles' do + expect do + create(:wiki_page_meta, project: project, title: FFaker::Lorem.characters(300)) + end.to raise_error(ActiveRecord::ValueTooLong) + end + + it 'is forbidden to have two records for the same project with the same canonical_slug' do + the_slug = generate(:sluggified_title) + create(:wiki_page_meta, canonical_slug: the_slug, project: project) + + in_violation = build(:wiki_page_meta, canonical_slug: the_slug, project: project) + + expect(in_violation).not_to be_valid + end + end + + describe '#canonical_slug' do + subject { described_class.find(meta.id) } + + let_it_be(:meta) do + described_class.create(title: generate(:wiki_page_title), project: project) + end + + context 'there are no slugs' do + it { is_expected.to have_attributes(canonical_slug: be_nil) } + end + + it 'can be set on initialization' do + meta = create(:wiki_page_meta, canonical_slug: 'foo') + + expect(meta.canonical_slug).to eq('foo') + end + + context 'we have some non-canonical slugs' do + before do + create_list(:wiki_page_slug, 2, wiki_page_meta: subject) + end + + it { is_expected.to have_attributes(canonical_slug: be_nil) } + + it 'issues at most one query' do + expect { subject.canonical_slug }.not_to exceed_query_limit(1) + end + + it 'issues no queries if we already know the slug' do + subject.canonical_slug + + expect { subject.canonical_slug }.not_to exceed_query_limit(0) + end + end + + context 'we have a canonical slug' do + before do + create_list(:wiki_page_slug, 2, wiki_page_meta: subject) + end + + it 'has the correct value' do + slug = create(:wiki_page_slug, :canonical, wiki_page_meta: subject) + + is_expected.to have_attributes(canonical_slug: slug.slug) + end + end + + describe 'canonical_slug=' do + shared_examples 'canonical_slug setting examples' do + # Constant overhead of two queries for the transaction + let(:upper_query_limit) { query_limit + 2 } + let(:lower_query_limit) { [upper_query_limit - 1, 0].max} + let(:other_slug) { generate(:sluggified_title) } + + it 'changes it to the correct value' do + subject.canonical_slug = slug + + expect(subject).to have_attributes(canonical_slug: slug) + end + + it 'ensures the slug is in the db' do + subject.canonical_slug = slug + + expect(subject.slugs.canonical.where(slug: slug)).to exist + end + + it 'issues at most N queries' do + expect { subject.canonical_slug = slug }.not_to exceed_query_limit(upper_query_limit) + end + + it 'issues fewer queries if we already know the current slug' do + subject.canonical_slug = other_slug + + expect { subject.canonical_slug = slug }.not_to exceed_query_limit(lower_query_limit) + end + end + + context 'the slug is not known to us' do + let(:slug) { generate(:sluggified_title) } + let(:query_limit) { 8 } + + include_examples 'canonical_slug setting examples' + end + + context 'the slug is already in the DB (but not canonical)' do + let_it_be(:slug_record) { create(:wiki_page_slug, wiki_page_meta: meta) } + let(:slug) { slug_record.slug } + let(:query_limit) { 4 } + + include_examples 'canonical_slug setting examples' + end + + context 'the slug is already in the DB (and canonical)' do + let_it_be(:slug_record) { create(:wiki_page_slug, :canonical, wiki_page_meta: meta) } + let(:slug) { slug_record.slug } + let(:query_limit) { 4 } + + include_examples 'canonical_slug setting examples' + end + + context 'the slug is up to date and in the DB' do + let(:slug) { generate(:sluggified_title) } + + before do + subject.canonical_slug = slug + end + + include_examples 'canonical_slug setting examples' do + let(:other_slug) { slug } + let(:upper_query_limit) { 0 } + end + end + end + end + + describe '.find_or_create' do + let(:old_title) { generate(:wiki_page_title) } + let(:last_known_slug) { generate(:sluggified_title) } + let(:current_slug) { wiki_page.slug } + let(:title) { wiki_page.title } + let(:wiki_page) { create(:wiki_page, project: project) } + + def find_record + described_class.find_or_create(last_known_slug, wiki_page) + end + + def create_previous_version(title = old_title, slug = last_known_slug) + create(:wiki_page_meta, title: title, project: project, canonical_slug: slug) + end + + def create_context + # Ensure that we behave nicely with respect to other projects + # We have: + # - page in other project with same canonical_slug + create(:wiki_page_meta, project: other_project, canonical_slug: wiki_page.slug) + + # - page in same project with different canonical_slug, but with + # an old slug that = canonical_slug + different_slug = generate(:sluggified_title) + create(:wiki_page_meta, project: project, canonical_slug: different_slug) + .slugs.create(slug: wiki_page.slug) + end + + shared_examples 'metadata examples' do + it 'establishes the correct state', :aggregate_failures do + create_context + + meta = find_record + + expect(meta).to have_attributes( + valid?: true, + canonical_slug: wiki_page.slug, + title: wiki_page.title, + project: wiki_page.wiki.project + ) + expect(meta.slugs.where(slug: last_known_slug)).to exist + expect(meta.slugs.canonical.where(slug: wiki_page.slug)).to exist + end + + it 'makes a reasonable number of DB queries' do + expect(project).to eq(wiki_page.wiki.project) + + expect { find_record }.not_to exceed_query_limit(query_limit) + end + end + + context 'the slug is too long' do + let(:last_known_slug) { FFaker::Lorem.characters(2050) } + + it 'raises an error' do + expect { find_record }.to raise_error ActiveRecord::ValueTooLong + end + end + + context 'a conflicting record exists' do + before do + create(:wiki_page_meta, project: project, canonical_slug: last_known_slug) + create(:wiki_page_meta, project: project, canonical_slug: current_slug) + end + + it 'raises an error' do + expect { find_record }.to raise_error(ActiveRecord::RecordInvalid) + end + end + + context 'no existing record exists' do + include_examples 'metadata examples' do + # The base case is 5 queries: + # - 2 for the outer transaction + # - 1 to find the metadata object if it exists + # - 1 to create it if it does not + # - 1 to insert last_known_slug and current_slug + # + # (Log has been edited for clarity) + # SAVEPOINT active_record_2 + # + # SELECT * FROM wiki_page_meta + # INNER JOIN wiki_page_slugs + # ON wiki_page_slugs.wiki_page_meta_id = wiki_page_meta.id + # WHERE wiki_page_meta.project_id = ? + # AND wiki_page_slugs.canonical = TRUE + # AND wiki_page_slugs.slug IN (?,?) + # LIMIT 2 + # + # INSERT INTO wiki_page_meta (project_id, title) VALUES (?, ?) RETURNING id + # + # INSERT INTO wiki_page_slugs (wiki_page_meta_id,slug,canonical) + # VALUES (?, ?, ?) (?, ?, ?) + # ON CONFLICT DO NOTHING RETURNING id + # + # RELEASE SAVEPOINT active_record_2 + let(:query_limit) { 5 } + end + end + + context 'the last_known_slug is the same as the current slug, as on creation' do + let(:last_known_slug) { current_slug } + + include_examples 'metadata examples' do + # Identical to the base case. + let(:query_limit) { 5 } + end + end + + context 'a record exists in the DB in the correct state' do + let(:last_known_slug) { current_slug } + let(:old_title) { title } + + before do + create_previous_version + end + + include_examples 'metadata examples' do + # We just need to do the initial query, and the outer transaction + # SAVEPOINT active_record_2 + # + # SELECT * FROM wiki_page_meta + # INNER JOIN wiki_page_slugs + # ON wiki_page_slugs.wiki_page_meta_id = wiki_page_meta.id + # WHERE wiki_page_meta.project_id = ? + # AND wiki_page_slugs.canonical = TRUE + # AND wiki_page_slugs.slug = ? + # LIMIT 2 + # + # RELEASE SAVEPOINT active_record_2 + let(:query_limit) { 3 } + end + end + + context 'we need to update the slug, but not the title' do + let(:old_title) { title } + + before do + create_previous_version + end + + include_examples 'metadata examples' do + # Here we need: + # - 2 for the outer transaction + # - 1 to find the record + # - 1 to insert the new slug + # - 3 to set canonical state correctly + # + # SAVEPOINT active_record_2 + # + # SELECT * FROM wiki_page_meta + # INNER JOIN wiki_page_slugs + # ON wiki_page_slugs.wiki_page_meta_id = wiki_page_meta.id + # WHERE wiki_page_meta.project_id = ? + # AND wiki_page_slugs.canonical = TRUE + # AND wiki_page_slugs.slug = ? + # LIMIT 1 + # + # INSERT INTO wiki_page_slugs (wiki_page_meta_id,slug,canonical) + # VALUES (?, ?, ?) ON CONFLICT DO NOTHING RETURNING id + # + # SELECT * FROM wiki_page_slugs + # WHERE wiki_page_slugs.wiki_page_meta_id = ? + # AND wiki_page_slugs.slug = ? + # LIMIT 1 + # UPDATE wiki_page_slugs SET canonical = FALSE WHERE wiki_page_meta_id = ? + # UPDATE wiki_page_slugs SET canonical = TRUE WHERE id = ? + # + # RELEASE SAVEPOINT active_record_2 + let(:query_limit) { 7 } + end + end + + context 'we need to update the title, but not the slug' do + let(:last_known_slug) { wiki_page.slug } + + before do + create_previous_version + end + + include_examples 'metadata examples' do + # Same as minimal case, plus one query to update the title. + # + # SAVEPOINT active_record_2 + # + # SELECT * FROM wiki_page_meta + # INNER JOIN wiki_page_slugs + # ON wiki_page_slugs.wiki_page_meta_id = wiki_page_meta.id + # WHERE wiki_page_meta.project_id = ? + # AND wiki_page_slugs.canonical = TRUE + # AND wiki_page_slugs.slug = ? + # LIMIT 1 + # + # UPDATE wiki_page_meta SET title = ? WHERE id = ? + # + # RELEASE SAVEPOINT active_record_2 + let(:query_limit) { 4 } + end + end + + context 'we want to change the slug back to a previous version' do + let(:slug_1) { 'foo' } + let(:slug_2) { 'bar' } + + let(:wiki_page) { create(:wiki_page, title: slug_1, project: project) } + let(:last_known_slug) { slug_2 } + + before do + meta = create_previous_version(title, slug_1) + meta.canonical_slug = slug_2 + end + + include_examples 'metadata examples' do + let(:query_limit) { 7 } + end + end + + context 'we want to change the slug a bunch of times' do + let(:slugs) { generate_list(:sluggified_title, 3) } + + before do + meta = create_previous_version + slugs.each { |slug| meta.canonical_slug = slug } + end + + include_examples 'metadata examples' do + let(:query_limit) { 7 } + end + end + + context 'we need to update the title and the slug' do + before do + create_previous_version + end + + include_examples 'metadata examples' do + # -- outer transaction + # SAVEPOINT active_record_2 + # + # -- to find the record + # SELECT * FROM wiki_page_meta + # INNER JOIN wiki_page_slugs + # ON wiki_page_slugs.wiki_page_meta_id = wiki_page_meta.id + # WHERE wiki_page_meta.project_id = ? + # AND wiki_page_slugs.canonical = TRUE + # AND wiki_page_slugs.slug IN (?,?) + # LIMIT 2 + # + # -- to update the title + # UPDATE wiki_page_meta SET title = ? WHERE id = ? + # + # -- to update slug + # INSERT INTO wiki_page_slugs (wiki_page_meta_id,slug,canonical) + # VALUES (?, ?, ?) ON CONFLICT DO NOTHING RETURNING id + # + # UPDATE wiki_page_slugs SET canonical = FALSE WHERE wiki_page_meta_id = ? + # + # SELECT * FROM wiki_page_slugs + # WHERE wiki_page_slugs.wiki_page_meta_id = ? + # AND wiki_page_slugs.slug = ? + # LIMIT 1 + # + # UPDATE wiki_page_slugs SET canonical = TRUE WHERE id = ? + # + # RELEASE SAVEPOINT active_record_2 + let(:query_limit) { 8 } + end + end + end +end diff --git a/spec/models/wiki_page/slug_spec.rb b/spec/models/wiki_page/slug_spec.rb new file mode 100644 index 00000000000..324dea6b320 --- /dev/null +++ b/spec/models/wiki_page/slug_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe WikiPage::Slug do + let_it_be(:meta) { create(:wiki_page_meta) } + + describe 'Associations' do + it { is_expected.to belong_to(:wiki_page_meta) } + + it 'refers correctly to the wiki_page_meta' do + created = create(:wiki_page_slug, wiki_page_meta: meta) + + expect(created.reload.wiki_page_meta).to eq(meta) + end + end + + describe 'scopes' do + describe 'canonical' do + subject { described_class.canonical } + + context 'there are no slugs' do + it { is_expected.to be_empty } + end + + context 'there are some non-canonical slugs' do + before do + create(:wiki_page_slug) + end + + it { is_expected.to be_empty } + end + + context 'there is at least one canonical slugs' do + before do + create(:wiki_page_slug, :canonical) + end + + it { is_expected.not_to be_empty } + end + end + end + + describe 'Validations' do + let(:canonical) { false } + + subject do + build(:wiki_page_slug, canonical: canonical, wiki_page_meta: meta) + end + + it { is_expected.to validate_presence_of(:slug) } + it { is_expected.to validate_uniqueness_of(:slug).scoped_to(:wiki_page_meta_id) } + + describe 'only_one_slug_can_be_canonical_per_meta_record' do + context 'there are no other slugs' do + it { is_expected.to be_valid } + + context 'the current slug is canonical' do + let(:canonical) { true } + + it { is_expected.to be_valid } + end + end + + context 'there are other slugs, but they are not canonical' do + before do + create(:wiki_page_slug, wiki_page_meta: meta) + end + + it { is_expected.to be_valid } + + context 'the current slug is canonical' do + let(:canonical) { true } + + it { is_expected.to be_valid } + end + end + + context 'there is already a canonical slug' do + before do + create(:wiki_page_slug, canonical: true, wiki_page_meta: meta) + end + + it { is_expected.to be_valid } + + context 'the current slug is canonical' do + let(:canonical) { true } + + it { is_expected.not_to be_valid } + end + end + end + end +end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 42a7d567613..e8e80a8c7f4 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -606,12 +606,36 @@ describe WikiPage do expect(subject).to eq(subject) end - it 'returns false for updated wiki page' do + it 'returns true for updated wiki page' do subject.update(content: "Updated content") - updated_page = wiki.find_page('test page') + updated_page = wiki.find_page(existing_page.slug) expect(updated_page).not_to be_nil - expect(updated_page).not_to eq(subject) + expect(updated_page).to eq(subject) + end + + it 'returns false for a completely different wiki page' do + other_page = create(:wiki_page) + + expect(subject.slug).not_to eq(other_page.slug) + expect(subject.project).not_to eq(other_page.project) + expect(subject).not_to eq(other_page) + end + + it 'returns false for page with different slug on same project' do + other_page = create(:wiki_page, project: subject.project) + + expect(subject.slug).not_to eq(other_page.slug) + expect(subject.project).to eq(other_page.project) + expect(subject).not_to eq(other_page) + end + + it 'returns false for page with the same slug on a different project' do + other_page = create(:wiki_page, title: existing_page.slug) + + expect(subject.slug).to eq(other_page.slug) + expect(subject.project).not_to eq(other_page.project) + expect(subject).not_to eq(other_page) end end diff --git a/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb b/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb index 431b0db392a..6f7250037cd 100644 --- a/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb @@ -83,7 +83,7 @@ describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_stor allow(::Gitlab::Metrics::Dashboard::Processor).to receive(:new).and_return(double(process: file_content_hash)) end - it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH, [STAGES::CommonMetricsInserter, STAGES::ProjectMetricsInserter, STAGES::Sorter] + it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH, [STAGES::CommonMetricsInserter, STAGES::CustomMetricsInserter, STAGES::Sorter] context 'selected branch already exists' do let(:branch) { 'existing_branch' } -- cgit v1.2.3