diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-07 09:08:57 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-07 09:08:57 +0000 |
commit | c417764f00abaa5d2224a50b8d43a15e40ef8790 (patch) | |
tree | de2134eec07b27df1770fad10bcd6aa3a52d8f90 | |
parent | cceb99c072e1eac3f144b479ea5909384fa39e7f (diff) | |
download | gitlab-ce-c417764f00abaa5d2224a50b8d43a15e40ef8790.tar.gz |
Add latest changes from gitlab-org/gitlab@master
43 files changed, 747 insertions, 277 deletions
diff --git a/app/assets/javascripts/alert_management/components/alert_status.vue b/app/assets/javascripts/alert_management/components/alert_status.vue index c26dc7b63a0..c44397d7fe2 100644 --- a/app/assets/javascripts/alert_management/components/alert_status.vue +++ b/app/assets/javascripts/alert_management/components/alert_status.vue @@ -88,7 +88,7 @@ export default { @keydown.esc.native="$emit('hide-dropdown')" @hide="$emit('hide-dropdown')" > - <div class="dropdown-title text-center"> + <div v-if="isSidebar" class="dropdown-title text-center"> <span class="alert-title">{{ s__('AlertManagement|Assign status') }}</span> <gl-button :aria-label="__('Close')" 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 595baeeb14f..f9e0c232d7e 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 @@ -4,8 +4,11 @@ import Icon from '~/vue_shared/components/icon.vue'; import UserAvatarImage from '../user_avatar/user_avatar_image.vue'; import { glEmojiTag } from '../../../emoji'; +const MAX_SKELETON_LINES = 4; + export default { name: 'UserPopover', + maxSkeletonLines: MAX_SKELETON_LINES, components: { Icon, GlPopover, @@ -22,11 +25,6 @@ export default { required: true, default: null, }, - loaded: { - type: Boolean, - required: false, - default: false, - }, }, computed: { statusHtml() { @@ -42,14 +40,8 @@ export default { return ''; }, - nameIsLoading() { - return !this.user.name; - }, - workInformationIsLoading() { - return !this.user.loaded && this.user.workInformation === null; - }, - locationIsLoading() { - return !this.user.loaded && this.user.location === null; + userIsLoading() { + return !this.user?.loaded; }, }, }; @@ -63,49 +55,43 @@ export default { <user-avatar-image :img-src="user.avatarUrl" :size="60" css-classes="mr-2" /> </div> <div class="p-1 w-100"> - <h5 class="m-0"> - <span v-if="user.name">{{ user.name }}</span> - <gl-skeleton-loading v-else :lines="1" class="animation-container-small mb-1" /> - </h5> - <div class="text-secondary mb-2"> - <span v-if="user.username">@{{ user.username }}</span> - <gl-skeleton-loading v-else :lines="1" class="animation-container-small mb-1" /> - </div> - <div class="text-secondary"> - <div v-if="user.bio" class="d-flex mb-1"> - <icon name="profile" class="category-icon flex-shrink-0" /> - <span ref="bio" class="ml-1">{{ user.bio }}</span> - </div> - <div v-if="user.workInformation" class="d-flex mb-1"> - <icon - v-show="!workInformationIsLoading" - name="work" - class="category-icon flex-shrink-0" - /> - <span ref="workInformation" class="ml-1">{{ user.workInformation }}</span> - </div> + <template v-if="userIsLoading"> + <!-- `gl-skeleton-loading` does not support equal length lines --> + <!-- This can be migrated to `gl-skeleton-loader` when https://gitlab.com/gitlab-org/gitlab-ui/-/issues/872 is completed --> <gl-skeleton-loading - v-if="workInformationIsLoading" + v-for="n in $options.maxSkeletonLines" + :key="n" :lines="1" class="animation-container-small mb-1" /> - </div> - <div class="js-location text-secondary d-flex"> - <icon - v-show="!locationIsLoading && user.location" - name="location" - class="category-icon flex-shrink-0" - /> - <span v-if="user.location" class="ml-1">{{ user.location }}</span> - <gl-skeleton-loading - v-if="locationIsLoading" - :lines="1" - class="animation-container-small mb-1" - /> - </div> - <div v-if="statusHtml" class="js-user-status mt-2"> - <span v-html="statusHtml"></span> - </div> + </template> + <template v-else> + <div class="mb-2"> + <h5 class="m-0"> + {{ user.name }} + </h5> + <span class="text-secondary">@{{ user.username }}</span> + </div> + <div class="text-secondary"> + <div v-if="user.bio" class="d-flex mb-1"> + <icon name="profile" class="category-icon flex-shrink-0" /> + <span ref="bio" class="ml-1">{{ user.bio }}</span> + </div> + <div v-if="user.workInformation" class="d-flex mb-1"> + <icon name="work" class="category-icon flex-shrink-0" /> + <span ref="workInformation" class="ml-1">{{ user.workInformation }}</span> + </div> + </div> + <div class="js-location text-secondary d-flex"> + <div v-if="user.location"> + <icon name="location" class="category-icon flex-shrink-0" /> + <span class="ml-1">{{ user.location }}</span> + </div> + </div> + <div v-if="statusHtml" class="js-user-status mt-2"> + <span v-html="statusHtml"></span> + </div> + </template> </div> </div> </gl-popover> diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1baaf0b9ee8..789213a5fc6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -22,6 +22,7 @@ class ApplicationController < ActionController::Base include Impersonation include Gitlab::Logging::CloudflareHelper include Gitlab::Utils::StrongMemoize + include ControllerWithFeatureCategory before_action :authenticate_user!, except: [:route_not_found] before_action :enforce_terms!, if: :should_enforce_terms? diff --git a/app/controllers/concerns/controller_with_feature_category.rb b/app/controllers/concerns/controller_with_feature_category.rb new file mode 100644 index 00000000000..f8985cf0950 --- /dev/null +++ b/app/controllers/concerns/controller_with_feature_category.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module ControllerWithFeatureCategory + extend ActiveSupport::Concern + include Gitlab::ClassAttributes + + class_methods do + def feature_category(category, config = {}) + validate_config!(config) + + category_config = Config.new(category, config[:only], config[:except], config[:if], config[:unless]) + # Add the config to the beginning. That way, the last defined one takes precedence. + feature_category_configuration.unshift(category_config) + end + + def feature_category_for_action(action) + category_config = feature_category_configuration.find { |config| config.matches?(action) } + + category_config&.category || superclass_feature_category_for_action(action) + end + + private + + def validate_config!(config) + invalid_keys = config.keys - [:only, :except, :if, :unless] + if invalid_keys.any? + raise ArgumentError, "unknown arguments: #{invalid_keys} " + end + + if config.key?(:only) && config.key?(:except) + raise ArgumentError, "cannot configure both `only` and `except`" + end + end + + def feature_category_configuration + class_attributes[:feature_category_config] ||= [] + end + + def superclass_feature_category_for_action(action) + return unless superclass.respond_to?(:feature_category_for_action) + + superclass.feature_category_for_action(action) + end + end +end diff --git a/app/controllers/concerns/controller_with_feature_category/config.rb b/app/controllers/concerns/controller_with_feature_category/config.rb new file mode 100644 index 00000000000..624691ee4f6 --- /dev/null +++ b/app/controllers/concerns/controller_with_feature_category/config.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module ControllerWithFeatureCategory + class Config + attr_reader :category + + def initialize(category, only, except, if_proc, unless_proc) + @category = category.to_sym + @only, @except = only&.map(&:to_s), except&.map(&:to_s) + @if_proc, @unless_proc = if_proc, unless_proc + end + + def matches?(action) + included?(action) && !excluded?(action) && + if_proc?(action) && !unless_proc?(action) + end + + private + + attr_reader :only, :except, :if_proc, :unless_proc + + def if_proc?(action) + if_proc.nil? || if_proc.call(action) + end + + def unless_proc?(action) + unless_proc.present? && unless_proc.call(action) + end + + def included?(action) + only.nil? || only.include?(action) + end + + def excluded?(action) + except.present? && except.include?(action) + end + end +end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 20ef14e8546..1aa6002e9c5 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -45,6 +45,13 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions] + feature_category :source_code_management, + unless: -> (action) { action.ends_with?("_reports") } + feature_category :code_testing, + only: [:test_reports, :coverage_reports, :terraform_reports] + feature_category :accessibility_testing, + only: [:accessibility_reports] + def index @merge_requests = @issuables diff --git a/app/presenters/alert_management/alert_presenter.rb b/app/presenters/alert_management/alert_presenter.rb index efd403aa21c..a515c70152d 100644 --- a/app/presenters/alert_management/alert_presenter.rb +++ b/app/presenters/alert_management/alert_presenter.rb @@ -60,6 +60,7 @@ module AlertManagement metadata << list_item('Service', service) if service metadata << list_item('Monitoring tool', monitoring_tool) if monitoring_tool metadata << list_item('Hosts', host_links) if hosts.any? + metadata << list_item('Description', description) if description.present? metadata.join(MARKDOWN_LINE_BREAK) end diff --git a/app/workers/concerns/worker_attributes.rb b/app/workers/concerns/worker_attributes.rb index b19217b15de..2efa703684c 100644 --- a/app/workers/concerns/worker_attributes.rb +++ b/app/workers/concerns/worker_attributes.rb @@ -2,6 +2,7 @@ module WorkerAttributes extend ActiveSupport::Concern + include Gitlab::ClassAttributes # Resource boundaries that workers can declare through the # `resource_boundary` attribute @@ -30,24 +31,24 @@ module WorkerAttributes }.stringify_keys.freeze class_methods do - def feature_category(value) + def feature_category(value, *extras) raise "Invalid category. Use `feature_category_not_owned!` to mark a worker as not owned" if value == :not_owned - worker_attributes[:feature_category] = value + class_attributes[:feature_category] = value end # Special case: mark this work as not associated with a feature category # this should be used for cross-cutting concerns, such as mailer workers. def feature_category_not_owned! - worker_attributes[:feature_category] = :not_owned + class_attributes[:feature_category] = :not_owned end def get_feature_category - get_worker_attribute(:feature_category) + get_class_attribute(:feature_category) end def feature_category_not_owned? - get_worker_attribute(:feature_category) == :not_owned + get_feature_category == :not_owned end # This should be set to :high for jobs that need to be run @@ -61,11 +62,11 @@ module WorkerAttributes def urgency(urgency) raise "Invalid urgency: #{urgency}" unless VALID_URGENCIES.include?(urgency) - worker_attributes[:urgency] = urgency + class_attributes[:urgency] = urgency end def get_urgency - worker_attributes[:urgency] || :low + class_attributes[:urgency] || :low end # Set this attribute on a job when it will call to services outside of the @@ -73,85 +74,64 @@ module WorkerAttributes # doc/development/sidekiq_style_guide.md#Jobs-with-External-Dependencies for # details def worker_has_external_dependencies! - worker_attributes[:external_dependencies] = true + class_attributes[:external_dependencies] = true end # Returns a truthy value if the worker has external dependencies. # See doc/development/sidekiq_style_guide.md#Jobs-with-External-Dependencies # for details def worker_has_external_dependencies? - worker_attributes[:external_dependencies] + class_attributes[:external_dependencies] end def worker_resource_boundary(boundary) raise "Invalid boundary" unless VALID_RESOURCE_BOUNDARIES.include? boundary - worker_attributes[:resource_boundary] = boundary + class_attributes[:resource_boundary] = boundary end def get_worker_resource_boundary - worker_attributes[:resource_boundary] || :unknown + class_attributes[:resource_boundary] || :unknown end def idempotent! - worker_attributes[:idempotent] = true + class_attributes[:idempotent] = true end def idempotent? - worker_attributes[:idempotent] + class_attributes[:idempotent] end def weight(value) - worker_attributes[:weight] = value + class_attributes[:weight] = value end def get_weight - worker_attributes[:weight] || + class_attributes[:weight] || NAMESPACE_WEIGHTS[queue_namespace] || 1 end def tags(*values) - worker_attributes[:tags] = values + class_attributes[:tags] = values end def get_tags - Array(worker_attributes[:tags]) + Array(class_attributes[:tags]) end def deduplicate(strategy, options = {}) - worker_attributes[:deduplication_strategy] = strategy - worker_attributes[:deduplication_options] = options + class_attributes[:deduplication_strategy] = strategy + class_attributes[:deduplication_options] = options end def get_deduplicate_strategy - worker_attributes[:deduplication_strategy] || + class_attributes[:deduplication_strategy] || Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DEFAULT_STRATEGY end def get_deduplication_options - worker_attributes[:deduplication_options] || {} - end - - protected - - # Returns a worker attribute declared on this class or its parent class. - # This approach allows declared attributes to be inherited by - # child classes. - def get_worker_attribute(name) - worker_attributes[name] || superclass_worker_attributes(name) - end - - private - - def worker_attributes - @attributes ||= {} - end - - def superclass_worker_attributes(name) - return unless superclass.include? WorkerAttributes - - superclass.get_worker_attribute(name) + class_attributes[:deduplication_options] || {} end end end diff --git a/changelogs/unreleased/217362-move-configure-stage-usage-activity-to-ce.yml b/changelogs/unreleased/217362-move-configure-stage-usage-activity-to-ce.yml new file mode 100644 index 00000000000..909ffa115f0 --- /dev/null +++ b/changelogs/unreleased/217362-move-configure-stage-usage-activity-to-ce.yml @@ -0,0 +1,5 @@ +--- +title: Move monitor stage usage activity to CE +merge_request: 36067 +author: +type: changed diff --git a/changelogs/unreleased/223714-harden-ci-pipelines-auto_devops-config_repository-usage-data.yml b/changelogs/unreleased/223714-harden-ci-pipelines-auto_devops-config_repository-usage-data.yml new file mode 100644 index 00000000000..b0a95d72a37 --- /dev/null +++ b/changelogs/unreleased/223714-harden-ci-pipelines-auto_devops-config_repository-usage-data.yml @@ -0,0 +1,5 @@ +--- +title: Rework hardening CI pipelines usage data queries with an index +merge_request: 35494 +author: +type: performance diff --git a/changelogs/unreleased/225649-add-description-to-alert-issue-body.yml b/changelogs/unreleased/225649-add-description-to-alert-issue-body.yml new file mode 100644 index 00000000000..bfb25c2392c --- /dev/null +++ b/changelogs/unreleased/225649-add-description-to-alert-issue-body.yml @@ -0,0 +1,5 @@ +--- +title: Fix rendering alert issue description field. +merge_request: 35862 +author: +type: fixed diff --git a/changelogs/unreleased/alert-list-dropdown-header.yml b/changelogs/unreleased/alert-list-dropdown-header.yml new file mode 100644 index 00000000000..27f8938ac3b --- /dev/null +++ b/changelogs/unreleased/alert-list-dropdown-header.yml @@ -0,0 +1,5 @@ +--- +title: Hide dropdown header on list view +merge_request: 35954 +author: +type: other diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index 1e27f9779e8..1dd6d484968 100644 --- a/danger/documentation/Dangerfile +++ b/danger/documentation/Dangerfile @@ -1,27 +1,33 @@ # frozen_string_literal: true +def gitlab_danger + @gitlab_danger ||= GitlabDanger.new(helper.gitlab_helper) +end + docs_paths_to_review = helper.changes_by_category[:docs] -unless docs_paths_to_review.empty? - message 'This merge request adds or changes files that require a review ' \ - 'from the Technical Writing team.' +return if docs_paths_to_review.empty? + +message 'This merge request adds or changes files that require a review ' \ + 'from the Technical Writing team.' + +return unless gitlab_danger.ci? - if GitlabDanger.new(helper.gitlab_helper).ci? - markdown(<<~MARKDOWN) - ## Documentation review +markdown(<<~MARKDOWN) + ## Documentation review - The following files require a review from a technical writer: + The following files require a review from a technical writer: - * #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")} + * #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")} - The review does not need to block merging this merge request. See the: + The review does not need to block merging this merge request. See the: - - [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) for the appropriate technical writer for this review. - - [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review. - MARKDOWN + - [Technical Writers assignments](https://about.gitlab.com/handbook/engineering/technical-writing/#designated-technical-writers) for the appropriate technical writer for this review. + - [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review. +MARKDOWN - unless gitlab.mr_labels.include?('documentation') - warn 'This merge request is missing the ~documentation label.' - end - end +unless gitlab.mr_labels.include?('documentation') + gitlab.api.update_merge_request(gitlab.mr_json['project_id'], + gitlab.mr_json['iid'], + labels: (gitlab.mr_labels + ['documentation']).join(',')) end diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index a70bc8f561c..64faae1d35b 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -62,6 +62,8 @@ changes = helper.changes_by_category # Ignore any files that are known but uncategorized. Prompt for any unknown files changes.delete(:none) +# To reinstate roulette for documentation, remove this line. +changes.delete(:docs) categories = changes.keys - [:unknown] # Ensure to spin for database reviewer/maintainer when ~database is applied (e.g. to review SQL queries) diff --git a/db/post_migrate/20200704143633_add_index_on_user_id_and_created_at_where_source_to_ci_pipelines.rb b/db/post_migrate/20200704143633_add_index_on_user_id_and_created_at_where_source_to_ci_pipelines.rb new file mode 100644 index 00000000000..d84b2b4cad3 --- /dev/null +++ b/db/post_migrate/20200704143633_add_index_on_user_id_and_created_at_where_source_to_ci_pipelines.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexOnUserIdAndCreatedAtWhereSourceToCiPipelines < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_pipelines, [:user_id, :created_at, :config_source] + end + + def down + remove_concurrent_index :ci_pipelines, [:user_id, :created_at, :config_source] + end +end diff --git a/db/structure.sql b/db/structure.sql index 2f03335a7fa..345d7d89bbb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18764,6 +18764,8 @@ CREATE INDEX index_ci_pipelines_on_project_idandrefandiddesc ON public.ci_pipeli CREATE INDEX index_ci_pipelines_on_status ON public.ci_pipelines USING btree (status); +CREATE INDEX index_ci_pipelines_on_user_id_and_created_at_and_config_source ON public.ci_pipelines USING btree (user_id, created_at, config_source); + CREATE INDEX index_ci_pipelines_on_user_id_and_created_at_and_source ON public.ci_pipelines USING btree (user_id, created_at, source); CREATE UNIQUE INDEX index_ci_refs_on_project_id_and_ref_path ON public.ci_refs USING btree (project_id, ref_path); @@ -23573,6 +23575,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200630110826 20200702123805 20200703154822 +20200704143633 20200706005325 \. diff --git a/doc/ci/review_apps/index.md b/doc/ci/review_apps/index.md index 67fac70c8de..283e4c69941 100644 --- a/doc/ci/review_apps/index.md +++ b/doc/ci/review_apps/index.md @@ -260,10 +260,6 @@ For example, in a Ruby application, you would need to have this script: Then, when your app is deployed via GitLab CI/CD, those variables should get replaced with their real values. -NOTE: **Note:** -Future enhancements [are planned](https://gitlab.com/gitlab-org/gitlab/-/issues/11322) -to make this process even easier. - ### Determining merge request ID The visual review tools retrieve the merge request ID from the `data-merge-request-id` diff --git a/doc/development/telemetry/usage_ping.md b/doc/development/telemetry/usage_ping.md index 7cb2eccd519..a269e9fcc05 100644 --- a/doc/development/telemetry/usage_ping.md +++ b/doc/development/telemetry/usage_ping.md @@ -648,6 +648,13 @@ appear to be associated to any of the services running, since they all appear to | `projects_slack_slash_active` | `usage_activity_by_stage` | `configure` | | EE | Unique projects with Slack '/' commands enabled | | `projects_with_prometheus_alerts` | `usage_activity_by_stage` | `configure` | | EE | Projects with Prometheus enabled and no alerts | | `deploy_keys` | `usage_activity_by_stage` | `create` | | | | +| `clusters` | `usage_activity_by_stage` | `monitor` | | CE+EE | | +| `clusters_applications_prometheus` | `usage_activity_by_stage` | `monitor` | | CE+EE | | +| `operations_dashboard_default_dashboard` | `usage_activity_by_stage` | `monitor` | | CE+EE | | +| `operations_dashboard_users_with_projects_added` | `usage_activity_by_stage` | `monitor` | | EE | | +| `projects_prometheus_active` | `usage_activity_by_stage` | `monitor` | | EE | | +| `projects_with_error_tracking_enabled` | `usage_activity_by_stage` | `monitor` | | EE | | +| `projects_with_tracing_enabled` | `usage_activity_by_stage` | `monitor` | | EE | | | `keys` | `usage_activity_by_stage` | `create` | | | | | `projects_jira_dvcs_server_active` | `usage_activity_by_stage` | `plan` | | | | | `service_desk_enabled_projects` | `usage_activity_by_stage` | `plan` | | | | diff --git a/doc/user/group/index.md b/doc/user/group/index.md index d1b8f52a21c..77ccf342e5a 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -554,7 +554,7 @@ Some domains cannot be restricted. These are the most popular public email domai To enable this feature: 1. Navigate to the group's **Settings > General** page. -1. Expand the **Permissions, LFS, 2FA** section, and enter the domain names into **Restrict membership by email** field. You can enter multiple domains by separating each domain with a comma (,). +1. Expand the **Permissions, LFS, 2FA** section, and enter the domain names into **Restrict membership by email** field. 1. Click **Save changes**. This will enable the domain-checking for all new users added to the group from this moment on. diff --git a/lib/gitlab/class_attributes.rb b/lib/gitlab/class_attributes.rb new file mode 100644 index 00000000000..6560c97b2e6 --- /dev/null +++ b/lib/gitlab/class_attributes.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Gitlab + module ClassAttributes + extend ActiveSupport::Concern + + class_methods do + protected + + # Returns an attribute declared on this class or its parent class. + # This approach allows declared attributes to be inherited by + # child classes. + def get_class_attribute(name) + class_attributes[name] || superclass_attributes(name) + end + + private + + def class_attributes + @class_attributes ||= {} + end + + def superclass_attributes(name) + return unless superclass.include? Gitlab::ClassAttributes + + superclass.get_class_attribute(name) + end + end + end +end diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index eecab29eda8..5f6de3b0a6a 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -102,8 +102,8 @@ module Gitlab }.freeze # First-match win, so be sure to put more specific regex at the top... CATEGORIES = { - %r{\Adoc/} => :none, # To reinstate roulette for documentation, set to `:docs`. - %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :none, # To reinstate roulette for documentation, set to `:docs`. + %r{\Adoc/} => :docs, + %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs, %r{\A(ee/)?app/(assets|views)/} => :frontend, %r{\A(ee/)?public/} => :frontend, diff --git a/lib/gitlab/metrics/background_transaction.rb b/lib/gitlab/metrics/background_transaction.rb index fe1722b1095..7b05ae29b02 100644 --- a/lib/gitlab/metrics/background_transaction.rb +++ b/lib/gitlab/metrics/background_transaction.rb @@ -9,7 +9,7 @@ module Gitlab end def labels - { controller: @worker_class.name, action: 'perform' } + { controller: @worker_class.name, action: 'perform', feature_category: @worker_class.try(:get_feature_category).to_s } end end end diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb index 13a5dfc0800..da06be9c79c 100644 --- a/lib/gitlab/metrics/transaction.rb +++ b/lib/gitlab/metrics/transaction.rb @@ -7,7 +7,7 @@ module Gitlab include Gitlab::Metrics::Methods # base labels shared among all transactions - BASE_LABELS = { controller: nil, action: nil }.freeze + BASE_LABELS = { controller: nil, action: nil, feature_category: nil }.freeze # labels that potentially contain sensitive information and will be filtered FILTERED_LABELS = [:branch, :path].freeze diff --git a/lib/gitlab/metrics/web_transaction.rb b/lib/gitlab/metrics/web_transaction.rb index fa17548723e..2064f9290d3 100644 --- a/lib/gitlab/metrics/web_transaction.rb +++ b/lib/gitlab/metrics/web_transaction.rb @@ -32,6 +32,10 @@ module Gitlab action = "#{controller.action_name}" + # Try to get the feature category, but don't fail when the controller is + # not an ApplicationController. + feature_category = controller.class.try(:feature_category_for_action, action).to_s + # Devise exposes a method called "request_format" that does the below. # However, this method is not available to all controllers (e.g. certain # Doorkeeper controllers). As such we use the underlying code directly. @@ -45,7 +49,7 @@ module Gitlab action = "#{action}.#{suffix}" end - { controller: controller.class.name, action: action } + { controller: controller.class.name, action: action, feature_category: feature_category } end def labels_from_endpoint @@ -61,7 +65,10 @@ module Gitlab if route path = endpoint_paths_cache[route.request_method][route.path] - { controller: 'Grape', action: "#{route.request_method} #{path}" } + + # Feature categories will be added for grape endpoints in + # https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/462 + { controller: 'Grape', action: "#{route.request_method} #{path}", feature_category: '' } end end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index e935d336f74..7c51f0d422e 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -490,9 +490,17 @@ module Gitlab {} end + # rubocop: disable CodeReuse/ActiveRecord def usage_activity_by_stage_monitor(time_period) - {} + { + clusters: distinct_count(::Clusters::Cluster.where(time_period), :user_id), + clusters_applications_prometheus: cluster_applications_user_distinct_count(::Clusters::Applications::Prometheus, time_period), + operations_dashboard_default_dashboard: count(::User.active.with_dashboard('operations').where(time_period), + start: user_minimum_id, + finish: user_maximum_id) + } end + # rubocop: enable CodeReuse/ActiveRecord def usage_activity_by_stage_package(time_period) {} diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5cfe5d0b39c..e7c891231ef 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1301,6 +1301,9 @@ msgstr "" msgid "Add" msgstr "" +msgid "Add \"%{value}\" to allowlist" +msgstr "" + msgid "Add %d issue" msgid_plural "Add %d issues" msgstr[0] "" @@ -9373,9 +9376,6 @@ msgstr "" msgid "Exactly one of %{attributes} is required" msgstr "" -msgid "Example: <code>acme.com,acme.co.in,acme.uk</code>." -msgstr "" - msgid "Example: @sub\\.company\\.com$" msgstr "" @@ -14918,7 +14918,7 @@ msgstr "" msgid "MrDeploymentActions|Stop environment" msgstr "" -msgid "Multiple domains are supported with comma delimiters." +msgid "Multiple domains are supported." msgstr "" msgid "Multiple issue boards" @@ -19639,7 +19639,7 @@ msgstr "" msgid "Restrict access by IP address" msgstr "" -msgid "Restrict membership by email" +msgid "Restrict membership by email domain" msgstr "" msgid "Restricts sign-ups for email addresses that match the given regex. See the %{supported_syntax_link_start}supported syntax%{supported_syntax_link_end} for more information." diff --git a/spec/controllers/concerns/controller_with_feature_category/config_spec.rb b/spec/controllers/concerns/controller_with_feature_category/config_spec.rb new file mode 100644 index 00000000000..9b8ffd2baab --- /dev/null +++ b/spec/controllers/concerns/controller_with_feature_category/config_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require "fast_spec_helper" +require "rspec-parameterized" +require_relative "../../../../app/controllers/concerns/controller_with_feature_category/config" + +RSpec.describe ControllerWithFeatureCategory::Config do + describe "#matches?" do + using RSpec::Parameterized::TableSyntax + + where(:only_actions, :except_actions, :if_proc, :unless_proc, :test_action, :expected) do + nil | nil | nil | nil | "action" | true + [:included] | nil | nil | nil | "action" | false + [:included] | nil | nil | nil | "included" | true + nil | [:excluded] | nil | nil | "excluded" | false + nil | nil | true | nil | "action" | true + [:included] | nil | true | nil | "action" | false + [:included] | nil | true | nil | "included" | true + nil | [:excluded] | true | nil | "excluded" | false + nil | nil | false | nil | "action" | false + [:included] | nil | false | nil | "action" | false + [:included] | nil | false | nil | "included" | false + nil | [:excluded] | false | nil | "excluded" | false + nil | nil | nil | true | "action" | false + [:included] | nil | nil | true | "action" | false + [:included] | nil | nil | true | "included" | false + nil | [:excluded] | nil | true | "excluded" | false + nil | nil | nil | false | "action" | true + [:included] | nil | nil | false | "action" | false + [:included] | nil | nil | false | "included" | true + nil | [:excluded] | nil | false | "excluded" | false + nil | nil | true | false | "action" | true + [:included] | nil | true | false | "action" | false + [:included] | nil | true | false | "included" | true + nil | [:excluded] | true | false | "excluded" | false + nil | nil | false | true | "action" | false + [:included] | nil | false | true | "action" | false + [:included] | nil | false | true | "included" | false + nil | [:excluded] | false | true | "excluded" | false + end + + with_them do + let(:config) do + if_to_proc = if_proc.nil? ? nil : -> (_) { if_proc } + unless_to_proc = unless_proc.nil? ? nil : -> (_) { unless_proc } + + described_class.new(:category, only_actions, except_actions, if_to_proc, unless_to_proc) + end + + specify { expect(config.matches?(test_action)).to be(expected) } + end + end +end diff --git a/spec/controllers/concerns/controller_with_feature_category_spec.rb b/spec/controllers/concerns/controller_with_feature_category_spec.rb new file mode 100644 index 00000000000..e603a7d14c4 --- /dev/null +++ b/spec/controllers/concerns/controller_with_feature_category_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_relative "../../../app/controllers/concerns/controller_with_feature_category" +require_relative "../../../app/controllers/concerns/controller_with_feature_category/config" + +RSpec.describe ControllerWithFeatureCategory do + describe ".feature_category_for_action" do + let(:base_controller) do + Class.new do + include ControllerWithFeatureCategory + end + end + + let(:controller) do + Class.new(base_controller) do + feature_category :baz + feature_category :foo, except: %w(update edit) + feature_category :bar, only: %w(index show) + feature_category :quux, only: %w(destroy) + feature_category :quuz, only: %w(destroy) + end + end + + let(:subclass) do + Class.new(controller) do + feature_category :qux, only: %w(index) + end + end + + it "is nil when nothing was defined" do + expect(base_controller.feature_category_for_action("hello")).to be_nil + end + + it "returns the expected category", :aggregate_failures do + expect(controller.feature_category_for_action("update")).to eq(:baz) + expect(controller.feature_category_for_action("hello")).to eq(:foo) + expect(controller.feature_category_for_action("index")).to eq(:bar) + end + + it "returns the closest match for categories defined in subclasses" do + expect(subclass.feature_category_for_action("index")).to eq(:qux) + expect(subclass.feature_category_for_action("show")).to eq(:bar) + end + + it "returns the last defined feature category when multiple match" do + expect(controller.feature_category_for_action("destroy")).to eq(:quuz) + end + + it "raises an error when using including and excluding the same action" do + expect do + Class.new(base_controller) do + feature_category :hello, only: [:world], except: [:world] + end + end.to raise_error(%r(cannot configure both `only` and `except`)) + end + + it "raises an error when using unknown arguments" do + expect do + Class.new(base_controller) do + feature_category :hello, hello: :world + end + end.to raise_error(%r(unknown arguments)) + end + end +end diff --git a/spec/controllers/every_controller_spec.rb b/spec/controllers/every_controller_spec.rb new file mode 100644 index 00000000000..4785ee9ed8f --- /dev/null +++ b/spec/controllers/every_controller_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe "Every controller" do + context "feature categories" do + let_it_be(:feature_categories) do + YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:to_sym).to_set + end + + let_it_be(:controller_actions) do + # This will return tuples of all controller actions defined in the routes + # Only for controllers inheriting ApplicationController + # Excluding controllers from gems (OAuth, Sidekiq) + Rails.application.routes.routes + .map { |route| route.required_defaults.presence } + .compact + .select { |route| route[:controller].present? && route[:action].present? } + .map { |route| [constantize_controller(route[:controller]), route[:action]] } + .reject { |route| route.first.nil? || !route.first.include?(ControllerWithFeatureCategory) } + end + + let_it_be(:routes_without_category) do + controller_actions.map do |controller, action| + "#{controller}##{action}" unless controller.feature_category_for_action(action) + end.compact + end + + it "has feature categories" do + pending("We'll work on defining categories for all controllers: "\ + "https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/463") + + expect(routes_without_category).to be_empty, "#{routes_without_category.first(10)} did not have a category" + end + + it "completed controllers don't get new routes without categories" do + completed_controllers = [Projects::MergeRequestsController].map(&:to_s) + + newly_introduced_missing_category = routes_without_category.select do |route| + completed_controllers.any? { |controller| route.start_with?(controller) } + end + + expect(newly_introduced_missing_category).to be_empty + end + + it "recognizes the feature categories" do + routes_unknown_category = controller_actions.map do |controller, action| + used_category = controller.feature_category_for_action(action) + next unless used_category + next if used_category == :not_owned + + ["#{controller}##{action}", used_category] unless feature_categories.include?(used_category) + end.compact + + expect(routes_unknown_category).to be_empty, "#{routes_unknown_category.first(10)} had an unknown category" + end + + it "doesn't define or exclude categories on removed actions", :aggregate_failures do + controller_actions.group_by(&:first).each do |controller, controller_action| + existing_actions = controller_action.map(&:last) + used_actions = actions_defined_in_feature_category_config(controller) + non_existing_used_actions = used_actions - existing_actions + + expect(non_existing_used_actions).to be_empty, + "#{controller} used #{non_existing_used_actions} to define feature category, but the route does not exist" + end + end + end + + def constantize_controller(name) + "#{name.camelize}Controller".constantize + rescue NameError + nil # some controllers, like the omniauth ones are dynamic + end + + def actions_defined_in_feature_category_config(controller) + feature_category_configs = controller.send(:class_attributes)[:feature_category_config] + feature_category_configs.map do |config| + Array(config.send(:only)) + Array(config.send(:except)) + end.flatten.uniq.map(&:to_s) + end +end diff --git a/spec/frontend/alert_management/components/alert_management_list_spec.js b/spec/frontend/alert_management/components/alert_management_list_spec.js index ae994252657..ae95873cb1c 100644 --- a/spec/frontend/alert_management/components/alert_management_list_spec.js +++ b/spec/frontend/alert_management/components/alert_management_list_spec.js @@ -207,6 +207,15 @@ describe('AlertManagementList', () => { expect(findStatusDropdown().exists()).toBe(true); }); + it('does not display a dropdown status header', () => { + mountComponent({ + props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, + data: { alerts: { list: mockAlerts }, alertsCount, errored: false }, + loading: false, + }); + expect(findStatusDropdown().contains('.dropdown-title')).toBe(false); + }); + it('shows correct severity icons', () => { mountComponent({ props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, diff --git a/spec/frontend/alert_management/components/alert_sidebar_status_spec.js b/spec/frontend/alert_management/components/alert_sidebar_status_spec.js deleted file mode 100644 index 1c651bbacdc..00000000000 --- a/spec/frontend/alert_management/components/alert_sidebar_status_spec.js +++ /dev/null @@ -1,107 +0,0 @@ -import { mount } from '@vue/test-utils'; -import { GlDropdownItem, GlLoadingIcon } from '@gitlab/ui'; -import { trackAlertStatusUpdateOptions } from '~/alert_management/constants'; -import AlertSidebarStatus from '~/alert_management/components/sidebar/sidebar_status.vue'; -import updateAlertStatus from '~/alert_management/graphql/mutations/update_alert_status.mutation.graphql'; -import Tracking from '~/tracking'; -import mockAlerts from '../mocks/alerts.json'; - -const mockAlert = mockAlerts[0]; - -describe('Alert Details Sidebar Status', () => { - let wrapper; - const findStatusDropdownItem = () => wrapper.find(GlDropdownItem); - const findStatusLoadingIcon = () => wrapper.find(GlLoadingIcon); - - function mountComponent({ data, sidebarCollapsed = true, loading = false, stubs = {} } = {}) { - wrapper = mount(AlertSidebarStatus, { - propsData: { - alert: { ...mockAlert }, - ...data, - sidebarCollapsed, - projectPath: 'projectPath', - }, - mocks: { - $apollo: { - mutate: jest.fn(), - queries: { - alert: { - loading, - }, - }, - }, - }, - stubs, - }); - } - - afterEach(() => { - if (wrapper) { - wrapper.destroy(); - } - }); - - describe('updating the alert status', () => { - const mockUpdatedMutationResult = { - data: { - updateAlertStatus: { - errors: [], - alert: { - status: 'acknowledged', - }, - }, - }, - }; - - beforeEach(() => { - mountComponent({ - data: { alert: mockAlert }, - sidebarCollapsed: false, - loading: false, - }); - }); - - it('calls `$apollo.mutate` with `updateAlertStatus` mutation and variables containing `iid`, `status`, & `projectPath`', () => { - jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue(mockUpdatedMutationResult); - findStatusDropdownItem().vm.$emit('click'); - - expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({ - mutation: updateAlertStatus, - variables: { - iid: '1527542', - status: 'TRIGGERED', - projectPath: 'projectPath', - }, - }); - }); - - it('stops updating when the request fails', () => { - jest.spyOn(wrapper.vm.$apollo, 'mutate').mockReturnValue(Promise.reject(new Error())); - findStatusDropdownItem().vm.$emit('click'); - expect(findStatusLoadingIcon().exists()).toBe(false); - expect(wrapper.find('[data-testid="status"]').text()).toBe('Triggered'); - }); - }); - - describe('Snowplow tracking', () => { - beforeEach(() => { - jest.spyOn(Tracking, 'event'); - mountComponent({ - props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, - data: { alert: mockAlert }, - loading: false, - }); - }); - - it('should track alert status updates', () => { - Tracking.event.mockClear(); - jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({}); - findStatusDropdownItem().vm.$emit('click'); - const status = findStatusDropdownItem().text(); - setImmediate(() => { - const { category, action, label } = trackAlertStatusUpdateOptions; - expect(Tracking.event).toHaveBeenCalledWith(category, action, { label, property: status }); - }); - }); - }); -}); diff --git a/spec/frontend/alert_management/components/alert_managment_sidebar_assignees_spec.js b/spec/frontend/alert_management/components/sidebar/alert_managment_sidebar_assignees_spec.js index 7969f553b29..db086782424 100644 --- a/spec/frontend/alert_management/components/alert_managment_sidebar_assignees_spec.js +++ b/spec/frontend/alert_management/components/sidebar/alert_managment_sidebar_assignees_spec.js @@ -5,7 +5,7 @@ import { GlDropdownItem } from '@gitlab/ui'; import SidebarAssignee from '~/alert_management/components/sidebar/sidebar_assignee.vue'; import SidebarAssignees from '~/alert_management/components/sidebar/sidebar_assignees.vue'; import AlertSetAssignees from '~/alert_management/graphql/mutations/alert_set_assignees.mutation.graphql'; -import mockAlerts from '../mocks/alerts.json'; +import mockAlerts from '../../mocks/alerts.json'; const mockAlert = mockAlerts[0]; diff --git a/spec/frontend/alert_management/components/alert_sidebar_spec.js b/spec/frontend/alert_management/components/sidebar/alert_sidebar_spec.js index 2536e0c230a..5235ae63fee 100644 --- a/spec/frontend/alert_management/components/alert_sidebar_spec.js +++ b/spec/frontend/alert_management/components/sidebar/alert_sidebar_spec.js @@ -3,7 +3,7 @@ import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import AlertSidebar from '~/alert_management/components/alert_sidebar.vue'; import SidebarAssignees from '~/alert_management/components/sidebar/sidebar_assignees.vue'; -import mockAlerts from '../mocks/alerts.json'; +import mockAlerts from '../../mocks/alerts.json'; const mockAlert = mockAlerts[0]; diff --git a/spec/frontend/alert_management/components/sidebar/alert_sidebar_status_spec.js b/spec/frontend/alert_management/components/sidebar/alert_sidebar_status_spec.js new file mode 100644 index 00000000000..c2eaf540e9c --- /dev/null +++ b/spec/frontend/alert_management/components/sidebar/alert_sidebar_status_spec.js @@ -0,0 +1,129 @@ +import { mount } from '@vue/test-utils'; +import { GlDropdown, GlDropdownItem, GlLoadingIcon } from '@gitlab/ui'; +import { trackAlertStatusUpdateOptions } from '~/alert_management/constants'; +import AlertSidebarStatus from '~/alert_management/components/sidebar/sidebar_status.vue'; +import updateAlertStatus from '~/alert_management/graphql/mutations/update_alert_status.mutation.graphql'; +import Tracking from '~/tracking'; +import mockAlerts from '../../mocks/alerts.json'; + +const mockAlert = mockAlerts[0]; + +describe('Alert Details Sidebar Status', () => { + let wrapper; + const findStatusDropdown = () => wrapper.find(GlDropdown); + const findStatusDropdownItem = () => wrapper.find(GlDropdownItem); + const findStatusLoadingIcon = () => wrapper.find(GlLoadingIcon); + + function mountComponent({ data, sidebarCollapsed = true, loading = false, stubs = {} } = {}) { + wrapper = mount(AlertSidebarStatus, { + propsData: { + alert: { ...mockAlert }, + ...data, + sidebarCollapsed, + projectPath: 'projectPath', + }, + mocks: { + $apollo: { + mutate: jest.fn(), + queries: { + alert: { + loading, + }, + }, + }, + }, + stubs, + }); + } + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + describe('Alert Sidebar Dropdown Status', () => { + beforeEach(() => { + mountComponent({ + data: { alert: mockAlert }, + sidebarCollapsed: false, + loading: false, + }); + }); + + it('displays status dropdown', () => { + expect(findStatusDropdown().exists()).toBe(true); + }); + + it('displays the dropdown status header', () => { + expect(findStatusDropdown().contains('.dropdown-title')).toBe(true); + }); + + describe('updating the alert status', () => { + const mockUpdatedMutationResult = { + data: { + updateAlertStatus: { + errors: [], + alert: { + status: 'acknowledged', + }, + }, + }, + }; + + beforeEach(() => { + mountComponent({ + data: { alert: mockAlert }, + sidebarCollapsed: false, + loading: false, + }); + }); + + it('calls `$apollo.mutate` with `updateAlertStatus` mutation and variables containing `iid`, `status`, & `projectPath`', () => { + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue(mockUpdatedMutationResult); + findStatusDropdownItem().vm.$emit('click'); + + expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({ + mutation: updateAlertStatus, + variables: { + iid: '1527542', + status: 'TRIGGERED', + projectPath: 'projectPath', + }, + }); + }); + + it('stops updating when the request fails', () => { + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockReturnValue(Promise.reject(new Error())); + findStatusDropdownItem().vm.$emit('click'); + expect(findStatusLoadingIcon().exists()).toBe(false); + expect(wrapper.find('[data-testid="status"]').text()).toBe('Triggered'); + }); + }); + + describe('Snowplow tracking', () => { + beforeEach(() => { + jest.spyOn(Tracking, 'event'); + mountComponent({ + props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, + data: { alert: mockAlert }, + loading: false, + }); + }); + + it('should track alert status updates', () => { + Tracking.event.mockClear(); + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({}); + findStatusDropdownItem().vm.$emit('click'); + const status = findStatusDropdownItem().text(); + setImmediate(() => { + const { category, action, label } = trackAlertStatusUpdateOptions; + expect(Tracking.event).toHaveBeenCalledWith(category, action, { + label, + property: status, + }); + }); + }); + }); + }); +}); diff --git a/spec/frontend/alert_management/components/alert_management_system_note_spec.js b/spec/frontend/alert_management/components/system_notes/alert_management_system_note_spec.js index 87dc36cc7cb..652cbc4b838 100644 --- a/spec/frontend/alert_management/components/alert_management_system_note_spec.js +++ b/spec/frontend/alert_management/components/system_notes/alert_management_system_note_spec.js @@ -1,6 +1,6 @@ import { shallowMount } from '@vue/test-utils'; import SystemNote from '~/alert_management/components/system_notes/system_note.vue'; -import mockAlerts from '../mocks/alerts.json'; +import mockAlerts from '../../mocks/alerts.json'; const mockAlert = mockAlerts[1]; 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 2c7fce714f0..16aa8379f56 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 @@ -4,7 +4,6 @@ import UserPopover from '~/vue_shared/components/user_popover/user_popover.vue'; import Icon from '~/vue_shared/components/icon.vue'; const DEFAULT_PROPS = { - loaded: true, user: { username: 'root', name: 'Administrator', @@ -12,6 +11,7 @@ const DEFAULT_PROPS = { bio: null, workInformation: null, status: null, + loaded: true, }, }; @@ -46,28 +46,21 @@ describe('User Popover Component', () => { }); }; - describe('Empty', () => { - beforeEach(() => { - createWrapper( - {}, - { - propsData: { - target: findTarget(), - user: { - name: null, - username: null, - location: null, - bio: null, - workInformation: null, - status: null, - }, - }, + describe('when user is loading', () => { + it('displays skeleton loaders', () => { + createWrapper({ + user: { + name: null, + username: null, + location: null, + bio: null, + workInformation: null, + status: null, + loaded: false, }, - ); - }); + }); - it('should return skeleton loaders', () => { - expect(wrapper.find(GlSkeletonLoading).exists()).toBe(true); + expect(wrapper.findAll(GlSkeletonLoading)).toHaveLength(4); }); }); diff --git a/spec/lib/gitlab/class_attributes_spec.rb b/spec/lib/gitlab/class_attributes_spec.rb new file mode 100644 index 00000000000..f8766f20495 --- /dev/null +++ b/spec/lib/gitlab/class_attributes_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true +require 'fast_spec_helper' + +RSpec.describe Gitlab::ClassAttributes do + let(:klass) do + Class.new do + include Gitlab::ClassAttributes + + def self.get_attribute(name) + get_class_attribute(name) + end + + def self.set_attribute(name, value) + class_attributes[name] = value + end + end + end + + let(:subclass) { Class.new(klass) } + + describe ".get_class_attribute" do + it "returns values set on the class" do + klass.set_attribute(:foo, :bar) + + expect(klass.get_attribute(:foo)).to eq(:bar) + end + + it "returns values set on a superclass" do + klass.set_attribute(:foo, :bar) + + expect(subclass.get_attribute(:foo)).to eq(:bar) + end + + it "returns values from the subclass over attributes from a superclass" do + klass.set_attribute(:foo, :baz) + subclass.set_attribute(:foo, :bar) + + expect(subclass.get_attribute(:foo)).to eq(:bar) + end + end +end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index 9d6e80d12af..3508d02838c 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -167,13 +167,13 @@ RSpec.describe Gitlab::Danger::Helper do describe '#categories_for_file' do where(:path, :expected_categories) do - 'doc/foo' | [:none] - 'CONTRIBUTING.md' | [:none] - 'LICENSE' | [:none] - 'MAINTENANCE.md' | [:none] - 'PHILOSOPHY.md' | [:none] - 'PROCESS.md' | [:none] - 'README.md' | [:none] + 'doc/foo' | [:docs] + 'CONTRIBUTING.md' | [:docs] + 'LICENSE' | [:docs] + 'MAINTENANCE.md' | [:docs] + 'PHILOSOPHY.md' | [:docs] + 'PROCESS.md' | [:docs] + 'README.md' | [:docs] 'ee/doc/foo' | [:unknown] 'ee/README' | [:unknown] diff --git a/spec/lib/gitlab/metrics/background_transaction_spec.rb b/spec/lib/gitlab/metrics/background_transaction_spec.rb index e4306806e0e..640bbebf0da 100644 --- a/spec/lib/gitlab/metrics/background_transaction_spec.rb +++ b/spec/lib/gitlab/metrics/background_transaction_spec.rb @@ -9,7 +9,16 @@ RSpec.describe Gitlab::Metrics::BackgroundTransaction do describe '#label' do it 'returns labels based on class name' do - expect(subject.labels).to eq(controller: 'TestWorker', action: 'perform') + expect(subject.labels).to eq(controller: 'TestWorker', action: 'perform', feature_category: '') + end + + it 'contains only the labels defined for metrics' do + expect(subject.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys) + end + + it 'includes the feature category if there is one' do + expect(test_worker_class).to receive(:get_feature_category).and_return('source_code_management') + expect(subject.labels).to include(feature_category: 'source_code_management') end end end diff --git a/spec/lib/gitlab/metrics/web_transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb index 6ceba186660..12e98089066 100644 --- a/spec/lib/gitlab/metrics/web_transaction_spec.rb +++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb @@ -70,6 +70,9 @@ RSpec.describe Gitlab::Metrics::WebTransaction do end describe '#labels' do + let(:request) { double(:request, format: double(:format, ref: :html)) } + let(:controller_class) { double(:controller_class, name: 'TestController') } + context 'when request goes to Grape endpoint' do before do route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)') @@ -77,8 +80,13 @@ RSpec.describe Gitlab::Metrics::WebTransaction do env['api.endpoint'] = endpoint end + it 'provides labels with the method and path of the route in the grape endpoint' do - expect(transaction.labels).to eq({ controller: 'Grape', action: 'GET /projects/:id/archive' }) + expect(transaction.labels).to eq({ controller: 'Grape', action: 'GET /projects/:id/archive', feature_category: '' }) + end + + it 'contains only the labels defined for transactions' do + expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys) end it 'does not provide labels if route infos are missing' do @@ -92,24 +100,25 @@ RSpec.describe Gitlab::Metrics::WebTransaction do end context 'when request goes to ActionController' do - let(:request) { double(:request, format: double(:format, ref: :html)) } - before do - klass = double(:klass, name: 'TestController') - controller = double(:controller, class: klass, action_name: 'show', request: request) + controller = double(:controller, class: controller_class, action_name: 'show', request: request) env['action_controller.instance'] = controller end it 'tags a transaction with the name and action of a controller' do - expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' }) + end + + it 'contains only the labels defined for transactions' do + expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys) end context 'when the request content type is not :html' do let(:request) { double(:request, format: double(:format, ref: :json)) } it 'appends the mime type to the transaction action' do - expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json' }) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json', feature_category: '' }) end end @@ -117,7 +126,14 @@ RSpec.describe Gitlab::Metrics::WebTransaction do let(:request) { double(:request, format: double(:format, ref: 'http://example.com')) } it 'does not append the MIME type to the transaction action' do - expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' }) + end + end + + context 'when the feature category is known' do + it 'includes it in the feature category label' do + expect(controller_class).to receive(:feature_category_for_action).with('show').and_return(:source_code_management) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: "source_code_management" }) end end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index be761060b1b..cad99beaf20 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -93,6 +93,28 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end + context 'for monitor' do + it 'includes accurate usage_activity_by_stage data' do + for_defined_days_back do + user = create(:user, dashboard: 'operations') + cluster = create(:cluster, user: user) + create(:project, creator: user) + create(:clusters_applications_prometheus, :installed, cluster: cluster) + end + + expect(described_class.uncached_data[:usage_activity_by_stage][:monitor]).to include( + clusters: 2, + clusters_applications_prometheus: 2, + operations_dashboard_default_dashboard: 2 + ) + expect(described_class.uncached_data[:usage_activity_by_stage_monthly][:monitor]).to include( + clusters: 1, + clusters_applications_prometheus: 1, + operations_dashboard_default_dashboard: 1 + ) + end + end + it 'ensures recorded_at is set before any other usage data calculation' do %i(alt_usage_data redis_usage_data distinct_count count).each do |method| expect(described_class).not_to receive(method) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 4853fc2438e..2852a86eeaf 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -12,6 +12,8 @@ RSpec.describe MergeRequest do subject { create(:merge_request) } describe 'associations' do + subject { build_stubbed(:merge_request) } + it { is_expected.to belong_to(:target_project).class_name('Project') } it { is_expected.to belong_to(:source_project).class_name('Project') } it { is_expected.to belong_to(:merge_user).class_name("User") } diff --git a/spec/presenters/alert_management/alert_presenter_spec.rb b/spec/presenters/alert_management/alert_presenter_spec.rb index 9a92048c487..b1bf7029f3e 100644 --- a/spec/presenters/alert_management/alert_presenter_spec.rb +++ b/spec/presenters/alert_management/alert_presenter_spec.rb @@ -12,7 +12,7 @@ RSpec.describe AlertManagement::AlertPresenter do } end let_it_be(:alert) do - create(:alert_management_alert, :with_host, :with_service, :with_monitoring_tool, project: project, payload: generic_payload) + create(:alert_management_alert, :with_description, :with_host, :with_service, :with_monitoring_tool, project: project, payload: generic_payload) end subject(:presenter) { described_class.new(alert) } @@ -29,7 +29,8 @@ RSpec.describe AlertManagement::AlertPresenter do **Severity:** #{presenter.severity}#{markdown_line_break} **Service:** #{alert.service}#{markdown_line_break} **Monitoring tool:** #{alert.monitoring_tool}#{markdown_line_break} - **Hosts:** #{alert.hosts.join(' ')} + **Hosts:** #{alert.hosts.join(' ')}#{markdown_line_break} + **Description:** #{alert.description} #### Alert Details |