From 9f0d27648937cb04d685ca9207f5c45f3ac98010 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 4 Nov 2022 09:11:46 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/assets/javascripts/webhooks/index.js | 7 +- app/helpers/hooks_helper.rb | 2 +- app/models/user.rb | 2 + app/models/users/namespace_commit_email.rb | 14 + .../applications/prometheus_update_service.rb | 38 -- .../base_change_timebox_service.rb | 4 + .../change_milestone_service.rb | 6 + app/views/projects/branches/_branch.html.haml | 3 +- .../merge_requests/_code_dropdown.html.haml | 4 +- app/workers/cluster_update_app_worker.rb | 36 +- .../counts_28d/20220222215951_xmau_plan.yml | 1 + ...20220222215952_xmau_project_management.yml | 1 + .../20220222215955_users_work_items.yml | 1 + ...s_updating_work_item_milestone_monthly.yml | 25 ++ .../counts_7d/20220222215851_xmau_plan.yml | 1 + ...20220222215852_xmau_project_management.yml | 1 + .../20220222215855_users_work_items.yml | 1 + ...rs_updating_work_item_milestone_weekly.yml | 25 ++ db/docs/namespace_commit_emails.yml | 9 + ...21213216_create_namespace_commit_emails.rb | 14 + ...dd_namespace_commit_emails_namespace_fk.rb | 15 + ...21_add_namespace_commit_emails_email_fk.rb | 15 + db/schema_migrations/20221021213216 | 1 + db/schema_migrations/20221022213505 | 1 + db/schema_migrations/20221022213521 | 1 + db/structure.sql | 38 ++ doc/ci/resource_groups/index.md | 1 - .../testing_guide/frontend_testing.md | 330 +++++++++++++++++- doc/development/testing_guide/index.md | 5 + lib/gitlab/database/gitlab_schemas.yml | 1 + .../known_events/work_items.yml | 5 + .../work_item_activity_unique_counter.rb | 5 + qa/Gemfile | 2 +- qa/Gemfile.lock | 4 +- qa/qa/support/loglinking.rb | 2 +- qa/spec/resource/api_fabricator_spec.rb | 2 +- qa/spec/support/loglinking_spec.rb | 2 +- .../users/namespace_commit_emails.rb | 9 + .../ide/user_opens_merge_request_spec.rb | 2 +- spec/features/projects/branches_spec.rb | 13 + spec/helpers/hooks_helper_spec.rb | 4 +- ...k_items_activity_aggregated_metric_spec.rb | 3 + .../work_item_activity_unique_counter_spec.rb | 8 + .../users/namespace_commit_email_spec.rb | 21 ++ .../prometheus_update_service_spec.rb | 111 ------ .../change_milestone_service_spec.rb | 31 ++ .../workers/cluster_update_app_worker_spec.rb | 112 ------ 47 files changed, 622 insertions(+), 317 deletions(-) create mode 100644 app/models/users/namespace_commit_email.rb delete mode 100644 app/services/clusters/applications/prometheus_update_service.rb create mode 100644 config/metrics/counts_28d/20221031070329_users_updating_work_item_milestone_monthly.yml create mode 100644 config/metrics/counts_7d/20221031065930_users_updating_work_item_milestone_weekly.yml create mode 100644 db/docs/namespace_commit_emails.yml create mode 100644 db/migrate/20221021213216_create_namespace_commit_emails.rb create mode 100644 db/migrate/20221022213505_add_namespace_commit_emails_namespace_fk.rb create mode 100644 db/migrate/20221022213521_add_namespace_commit_emails_email_fk.rb create mode 100644 db/schema_migrations/20221021213216 create mode 100644 db/schema_migrations/20221022213505 create mode 100644 db/schema_migrations/20221022213521 create mode 100644 spec/factories/users/namespace_commit_emails.rb create mode 100644 spec/models/users/namespace_commit_email_spec.rb delete mode 100644 spec/services/clusters/applications/prometheus_update_service_spec.rb delete mode 100644 spec/workers/cluster_update_app_worker_spec.rb diff --git a/app/assets/javascripts/webhooks/index.js b/app/assets/javascripts/webhooks/index.js index d90680a9bac..7d04978280b 100644 --- a/app/assets/javascripts/webhooks/index.js +++ b/app/assets/javascripts/webhooks/index.js @@ -10,11 +10,6 @@ export default () => { const { url: initialUrl, urlVariables } = el.dataset; - // Convert the array of 'key' strings to array of { key } objects - const initialUrlVariables = urlVariables - ? JSON.parse(urlVariables)?.map((key) => ({ key })) - : undefined; - return new Vue({ el, name: 'WebhookFormRoot', @@ -22,7 +17,7 @@ export default () => { return createElement(FormUrlApp, { props: { initialUrl, - initialUrlVariables, + initialUrlVariables: JSON.parse(urlVariables), }, }); }, diff --git a/app/helpers/hooks_helper.rb b/app/helpers/hooks_helper.rb index 44ed61b8fde..921e30edbaa 100644 --- a/app/helpers/hooks_helper.rb +++ b/app/helpers/hooks_helper.rb @@ -4,7 +4,7 @@ module HooksHelper def webhook_form_data(hook) { url: hook.url, - url_variables: Gitlab::Json.dump(hook.url_variables.keys) + url_variables: Gitlab::Json.dump(hook.url_variables.keys.map { { key: _1 } }) } end diff --git a/app/models/user.rb b/app/models/user.rb index 9e839028e8d..b6dce68616f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -261,6 +261,8 @@ class User < ApplicationRecord has_many :resource_state_events, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent has_many :authored_events, class_name: 'Event', dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent + has_many :namespace_commit_emails + # # Validations # diff --git a/app/models/users/namespace_commit_email.rb b/app/models/users/namespace_commit_email.rb new file mode 100644 index 00000000000..4ec02f12717 --- /dev/null +++ b/app/models/users/namespace_commit_email.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Users + class NamespaceCommitEmail < ApplicationRecord + belongs_to :user + belongs_to :namespace + belongs_to :email + + validates :user, presence: true + validates :namespace, presence: true + validates :email, presence: true + validates :user_id, uniqueness: { scope: [:namespace_id] } + end +end diff --git a/app/services/clusters/applications/prometheus_update_service.rb b/app/services/clusters/applications/prometheus_update_service.rb deleted file mode 100644 index b8b50f06d72..00000000000 --- a/app/services/clusters/applications/prometheus_update_service.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -module Clusters - module Applications - # Deprecated, to be removed in %14.0 as part of https://gitlab.com/groups/gitlab-org/-/epics/4280 - class PrometheusUpdateService < BaseHelmService - attr_accessor :project - - def initialize(app, project) - super(app) - @project = project - end - - def execute - raise NotImplementedError, 'Externally installed prometheus should not be modified!' unless app.managed_prometheus? - - app.make_updating! - - helm_api.update(patch_command(values)) - - ::ClusterWaitForAppUpdateWorker.perform_in(::ClusterWaitForAppUpdateWorker::INTERVAL, app.name, app.id) - rescue ::Kubeclient::HttpError => ke - app.make_update_errored!("Kubernetes error: #{ke.message}") - rescue StandardError => e - app.make_update_errored!(e.message) - end - - private - - def values - PrometheusConfigService - .new(project, cluster, app) - .execute - .to_yaml - end - end - end -end diff --git a/app/services/resource_events/base_change_timebox_service.rb b/app/services/resource_events/base_change_timebox_service.rb index 372f1c9d816..ba7c9d90713 100644 --- a/app/services/resource_events/base_change_timebox_service.rb +++ b/app/services/resource_events/base_change_timebox_service.rb @@ -12,11 +12,15 @@ module ResourceEvents def execute create_event + track_event + resource.expire_note_etag_cache end private + def track_event; end + def create_event raise NotImplementedError end diff --git a/app/services/resource_events/change_milestone_service.rb b/app/services/resource_events/change_milestone_service.rb index 24935a3327a..a092d807d8f 100644 --- a/app/services/resource_events/change_milestone_service.rb +++ b/app/services/resource_events/change_milestone_service.rb @@ -13,6 +13,12 @@ module ResourceEvents private + def track_event + return unless resource.is_a?(WorkItem) + + Gitlab::UsageDataCounters::WorkItemActivityUniqueCounter.track_work_item_milestone_changed_action(author: user) + end + def create_event ResourceMilestoneEvent.create(build_resource_args) end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 12e9044ad24..51c218f40b9 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -46,4 +46,5 @@ = render 'projects/buttons/download', project: @project, ref: branch.name, pipeline: @refs_pipelines[branch.name], class: 'gl-vertical-align-top' - = render 'projects/branches/delete_branch_modal_button', project: @project, branch: branch, merged: merged + - if can?(current_user, :push_code, @project) + = render 'projects/branches/delete_branch_modal_button', project: @project, branch: branch, merged: merged diff --git a/app/views/projects/merge_requests/_code_dropdown.html.haml b/app/views/projects/merge_requests/_code_dropdown.html.haml index eecea5ae855..5c7fe56095c 100644 --- a/app/views/projects/merge_requests/_code_dropdown.html.haml +++ b/app/views/projects/merge_requests/_code_dropdown.html.haml @@ -16,12 +16,12 @@ = _('Check out branch') - if current_user %li.gl-new-dropdown-item - = link_to ide_merge_request_path(@merge_request), class: 'dropdown-item', data: { qa_selector: 'open_in_web_ide_button' } do + = link_to ide_merge_request_path(@merge_request), class: 'dropdown-item', target: '_blank', data: { qa_selector: 'open_in_web_ide_button' } do .gl-new-dropdown-item-text-wrapper = _('Open in Web IDE') - if Gitlab::CurrentSettings.gitpod_enabled && current_user&.gitpod_enabled %li.gl-new-dropdown-item - = link_to "#{Gitlab::CurrentSettings.gitpod_url}##{merge_request_url(@merge_request)}", class: 'dropdown-item' do + = link_to "#{Gitlab::CurrentSettings.gitpod_url}##{merge_request_url(@merge_request)}", target: '_blank', class: 'dropdown-item' do .gl-new-dropdown-item-text-wrapper = _('Open in Gitpod') %li.gl-new-dropdown-divider diff --git a/app/workers/cluster_update_app_worker.rb b/app/workers/cluster_update_app_worker.rb index 97fdec02ba4..7d997c0a293 100644 --- a/app/workers/cluster_update_app_worker.rb +++ b/app/workers/cluster_update_app_worker.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true # Deprecated, to be removed in %14.0 as part of https://gitlab.com/groups/gitlab-org/-/epics/4280 +# Also see https://gitlab.com/gitlab-org/gitlab/-/issues/366573 class ClusterUpdateAppWorker # rubocop:disable Scalability/IdempotentWorker UpdateAlreadyInProgressError = Class.new(StandardError) @@ -16,38 +17,5 @@ class ClusterUpdateAppWorker # rubocop:disable Scalability/IdempotentWorker LEASE_TIMEOUT = 10.minutes.to_i - def perform(app_name, app_id, project_id, scheduled_time) - @app_id = app_id - - try_obtain_lease do - execute(app_name, app_id, project_id, scheduled_time) - end - end - - private - - def execute(app_name, app_id, project_id, scheduled_time) - project = Project.find_by_id(project_id) - return unless project - - find_application(app_name, app_id) do |app| - update_prometheus(app, scheduled_time, project) - end - end - - def update_prometheus(app, scheduled_time, project) - return unless app.managed_prometheus? - return if app.updated_since?(scheduled_time) - return if app.update_in_progress? - - Clusters::Applications::PrometheusUpdateService.new(app, project).execute - end - - def lease_key - @lease_key ||= "#{self.class.name.underscore}-#{@app_id}" - end - - def lease_timeout - LEASE_TIMEOUT - end + def perform(app_name, app_id, project_id, scheduled_time); end end diff --git a/config/metrics/counts_28d/20220222215951_xmau_plan.yml b/config/metrics/counts_28d/20220222215951_xmau_plan.yml index bc6c40d502f..70cc27b801c 100644 --- a/config/metrics/counts_28d/20220222215951_xmau_plan.yml +++ b/config/metrics/counts_28d/20220222215951_xmau_plan.yml @@ -23,6 +23,7 @@ options: - users_updating_work_item_labels - users_updating_work_item_iteration - users_updating_weight_estimate + - users_updating_work_item_milestone data_category: optional distribution: - ce diff --git a/config/metrics/counts_28d/20220222215952_xmau_project_management.yml b/config/metrics/counts_28d/20220222215952_xmau_project_management.yml index 9bafbe059ab..13a943c972c 100644 --- a/config/metrics/counts_28d/20220222215952_xmau_project_management.yml +++ b/config/metrics/counts_28d/20220222215952_xmau_project_management.yml @@ -23,6 +23,7 @@ options: - users_updating_work_item_labels - users_updating_work_item_iteration - users_updating_weight_estimate + - users_updating_work_item_milestone data_category: optional distribution: - ce diff --git a/config/metrics/counts_28d/20220222215955_users_work_items.yml b/config/metrics/counts_28d/20220222215955_users_work_items.yml index a6b169730e8..cb3dc63035f 100644 --- a/config/metrics/counts_28d/20220222215955_users_work_items.yml +++ b/config/metrics/counts_28d/20220222215955_users_work_items.yml @@ -23,6 +23,7 @@ options: - users_updating_work_item_labels - users_updating_work_item_iteration - users_updating_weight_estimate + - users_updating_work_item_milestone data_category: optional distribution: - ce diff --git a/config/metrics/counts_28d/20221031070329_users_updating_work_item_milestone_monthly.yml b/config/metrics/counts_28d/20221031070329_users_updating_work_item_milestone_monthly.yml new file mode 100644 index 00000000000..02edb32765e --- /dev/null +++ b/config/metrics/counts_28d/20221031070329_users_updating_work_item_milestone_monthly.yml @@ -0,0 +1,25 @@ +--- +key_path: redis_hll_counters.work_items.users_updating_work_item_milestone_monthly +description: Unique users updating a work item's milestone +product_section: dev +product_stage: plan +product_group: project_management +product_category: team_planning +value_type: number +status: active +milestone: "15.6" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102495 +time_frame: 28d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +options: + events: + - users_updating_work_item_milestone +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate diff --git a/config/metrics/counts_7d/20220222215851_xmau_plan.yml b/config/metrics/counts_7d/20220222215851_xmau_plan.yml index b1776b614fb..4520652ddf1 100644 --- a/config/metrics/counts_7d/20220222215851_xmau_plan.yml +++ b/config/metrics/counts_7d/20220222215851_xmau_plan.yml @@ -23,6 +23,7 @@ options: - users_updating_work_item_labels - users_updating_work_item_iteration - users_updating_weight_estimate + - users_updating_work_item_milestone data_category: optional distribution: - ce diff --git a/config/metrics/counts_7d/20220222215852_xmau_project_management.yml b/config/metrics/counts_7d/20220222215852_xmau_project_management.yml index 6aa913a778e..59543e71dcb 100644 --- a/config/metrics/counts_7d/20220222215852_xmau_project_management.yml +++ b/config/metrics/counts_7d/20220222215852_xmau_project_management.yml @@ -23,6 +23,7 @@ options: - users_updating_work_item_labels - users_updating_work_item_iteration - users_updating_weight_estimate + - users_updating_work_item_milestone data_category: optional distribution: - ce diff --git a/config/metrics/counts_7d/20220222215855_users_work_items.yml b/config/metrics/counts_7d/20220222215855_users_work_items.yml index 952f15f3050..6dc426231e9 100644 --- a/config/metrics/counts_7d/20220222215855_users_work_items.yml +++ b/config/metrics/counts_7d/20220222215855_users_work_items.yml @@ -23,6 +23,7 @@ options: - users_updating_work_item_labels - users_updating_work_item_iteration - users_updating_weight_estimate + - users_updating_work_item_milestone data_category: optional distribution: - ce diff --git a/config/metrics/counts_7d/20221031065930_users_updating_work_item_milestone_weekly.yml b/config/metrics/counts_7d/20221031065930_users_updating_work_item_milestone_weekly.yml new file mode 100644 index 00000000000..a06f0a5bdbe --- /dev/null +++ b/config/metrics/counts_7d/20221031065930_users_updating_work_item_milestone_weekly.yml @@ -0,0 +1,25 @@ +--- +key_path: redis_hll_counters.work_items.users_updating_work_item_milestone_weekly +description: Unique users updating a work item's milestone +product_section: dev +product_stage: plan +product_group: project_management +product_category: team_planning +value_type: number +status: active +milestone: "15.6" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102495 +time_frame: 7d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +options: + events: + - users_updating_work_item_milestone +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate diff --git a/db/docs/namespace_commit_emails.yml b/db/docs/namespace_commit_emails.yml new file mode 100644 index 00000000000..d7e192f97f4 --- /dev/null +++ b/db/docs/namespace_commit_emails.yml @@ -0,0 +1,9 @@ +--- +table_name: namespace_commit_emails +classes: +- Users::NamespaceCommitEmail +feature_categories: +- source_code_management +description: User default email for commits from the GitLab UI +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101832 +milestone: '15.6' diff --git a/db/migrate/20221021213216_create_namespace_commit_emails.rb b/db/migrate/20221021213216_create_namespace_commit_emails.rb new file mode 100644 index 00000000000..07811bf7b75 --- /dev/null +++ b/db/migrate/20221021213216_create_namespace_commit_emails.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateNamespaceCommitEmails < Gitlab::Database::Migration[2.0] + def change + create_table :namespace_commit_emails do |t| + t.references :user, index: false, null: false, foreign_key: { on_delete: :cascade } + t.references :namespace, null: false + t.references :email, null: false + t.timestamps_with_timezone null: false + + t.index [:user_id, :namespace_id], unique: true + end + end +end diff --git a/db/migrate/20221022213505_add_namespace_commit_emails_namespace_fk.rb b/db/migrate/20221022213505_add_namespace_commit_emails_namespace_fk.rb new file mode 100644 index 00000000000..0c543b03397 --- /dev/null +++ b/db/migrate/20221022213505_add_namespace_commit_emails_namespace_fk.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddNamespaceCommitEmailsNamespaceFk < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :namespace_commit_emails, :namespaces, column: :namespace_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :namespace_commit_emails, column: :namespace_id + end + end +end diff --git a/db/migrate/20221022213521_add_namespace_commit_emails_email_fk.rb b/db/migrate/20221022213521_add_namespace_commit_emails_email_fk.rb new file mode 100644 index 00000000000..9dbde26475c --- /dev/null +++ b/db/migrate/20221022213521_add_namespace_commit_emails_email_fk.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddNamespaceCommitEmailsEmailFk < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :namespace_commit_emails, :emails, column: :email_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :namespace_commit_emails, column: :email_id + end + end +end diff --git a/db/schema_migrations/20221021213216 b/db/schema_migrations/20221021213216 new file mode 100644 index 00000000000..900a4f6701e --- /dev/null +++ b/db/schema_migrations/20221021213216 @@ -0,0 +1 @@ +defe6e66c98648ea7fb77d8001392bc707ec022f639d346c42d23fad10958856 \ No newline at end of file diff --git a/db/schema_migrations/20221022213505 b/db/schema_migrations/20221022213505 new file mode 100644 index 00000000000..4cf0b87eedf --- /dev/null +++ b/db/schema_migrations/20221022213505 @@ -0,0 +1 @@ +c48015b2ff6ad4b58bffaf5342247d890f6bd2388c467751654bc705f5eb53ed \ No newline at end of file diff --git a/db/schema_migrations/20221022213521 b/db/schema_migrations/20221022213521 new file mode 100644 index 00000000000..c3bb483debf --- /dev/null +++ b/db/schema_migrations/20221022213521 @@ -0,0 +1 @@ +739952c72f82b804b84d73107264804202ad102b425008d4dcb029c1f02e2118 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9dc33fab3fa..57b94946643 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17951,6 +17951,24 @@ CREATE TABLE namespace_ci_cd_settings ( allow_stale_runner_pruning boolean DEFAULT false NOT NULL ); +CREATE TABLE namespace_commit_emails ( + id bigint NOT NULL, + user_id bigint NOT NULL, + namespace_id bigint NOT NULL, + email_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL +); + +CREATE SEQUENCE namespace_commit_emails_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE namespace_commit_emails_id_seq OWNED BY namespace_commit_emails.id; + CREATE TABLE namespace_details ( namespace_id bigint NOT NULL, created_at timestamp with time zone, @@ -23974,6 +23992,8 @@ ALTER TABLE ONLY namespace_admin_notes ALTER COLUMN id SET DEFAULT nextval('name ALTER TABLE ONLY namespace_bans ALTER COLUMN id SET DEFAULT nextval('namespace_bans_id_seq'::regclass); +ALTER TABLE ONLY namespace_commit_emails ALTER COLUMN id SET DEFAULT nextval('namespace_commit_emails_id_seq'::regclass); + ALTER TABLE ONLY namespace_statistics ALTER COLUMN id SET DEFAULT nextval('namespace_statistics_id_seq'::regclass); ALTER TABLE ONLY namespaces ALTER COLUMN id SET DEFAULT nextval('namespaces_id_seq'::regclass); @@ -26037,6 +26057,9 @@ ALTER TABLE ONLY namespace_bans ALTER TABLE ONLY namespace_ci_cd_settings ADD CONSTRAINT namespace_ci_cd_settings_pkey PRIMARY KEY (namespace_id); +ALTER TABLE ONLY namespace_commit_emails + ADD CONSTRAINT namespace_commit_emails_pkey PRIMARY KEY (id); + ALTER TABLE ONLY namespace_details ADD CONSTRAINT namespace_details_pkey PRIMARY KEY (namespace_id); @@ -29679,6 +29702,12 @@ CREATE UNIQUE INDEX index_namespace_bans_on_namespace_id_and_user_id ON namespac CREATE INDEX index_namespace_bans_on_user_id ON namespace_bans USING btree (user_id); +CREATE INDEX index_namespace_commit_emails_on_email_id ON namespace_commit_emails USING btree (email_id); + +CREATE INDEX index_namespace_commit_emails_on_namespace_id ON namespace_commit_emails USING btree (namespace_id); + +CREATE UNIQUE INDEX index_namespace_commit_emails_on_user_id_and_namespace_id ON namespace_commit_emails USING btree (user_id, namespace_id); + CREATE UNIQUE INDEX index_namespace_root_storage_statistics_on_namespace_id ON namespace_root_storage_statistics USING btree (namespace_id); CREATE UNIQUE INDEX index_namespace_statistics_on_namespace_id ON namespace_statistics USING btree (namespace_id); @@ -32849,6 +32878,9 @@ ALTER TABLE ONLY user_namespace_callouts ALTER TABLE ONLY sbom_occurrences ADD CONSTRAINT fk_4b88e5b255 FOREIGN KEY (component_version_id) REFERENCES sbom_component_versions(id) ON DELETE CASCADE; +ALTER TABLE ONLY namespace_commit_emails + ADD CONSTRAINT fk_4d6ba63ba5 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY vulnerability_reads ADD CONSTRAINT fk_4f593f6c62 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; @@ -33233,6 +33265,9 @@ ALTER TABLE ONLY issue_assignees ALTER TABLE ONLY agent_project_authorizations ADD CONSTRAINT fk_b7fe9b4777 FOREIGN KEY (agent_id) REFERENCES cluster_agents(id) ON DELETE CASCADE; +ALTER TABLE ONLY namespace_commit_emails + ADD CONSTRAINT fk_b8d89d555e FOREIGN KEY (email_id) REFERENCES emails(id) ON DELETE CASCADE; + ALTER TABLE ONLY ci_trigger_requests ADD CONSTRAINT fk_b8ec8b7245 FOREIGN KEY (trigger_id) REFERENCES ci_triggers(id) ON DELETE CASCADE; @@ -34937,6 +34972,9 @@ ALTER TABLE ONLY packages_debian_project_distributions ALTER TABLE ONLY incident_management_oncall_shifts ADD CONSTRAINT fk_rails_df4feb286a FOREIGN KEY (rotation_id) REFERENCES incident_management_oncall_rotations(id) ON DELETE CASCADE; +ALTER TABLE ONLY namespace_commit_emails + ADD CONSTRAINT fk_rails_dfa4c104f5 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ALTER TABLE ONLY analytics_cycle_analytics_group_stages ADD CONSTRAINT fk_rails_dfb37c880d FOREIGN KEY (end_event_label_id) REFERENCES labels(id) ON DELETE CASCADE; diff --git a/doc/ci/resource_groups/index.md b/doc/ci/resource_groups/index.md index 92808a2806c..b46008abb07 100644 --- a/doc/ci/resource_groups/index.md +++ b/doc/ci/resource_groups/index.md @@ -171,7 +171,6 @@ deploy: include: deploy.gitlab-ci.yml strategy: depend resource_group: AWS-production - environment: production ``` ```yaml diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 256675e23b7..041b0f0a4f4 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -11,7 +11,7 @@ at GitLab. We use Jest for JavaScript unit and integration testing, and RSpec feature tests with Capybara for e2e (end-to-end) integration testing. Unit and feature tests need to be written for all new features. -Most of the time, you should use [RSpec](https://github.com/rspec/rspec-rails#feature-specs) for your feature tests. +Most of the time, you should use [RSpec](https://github.com/rspec/rspec-rails#feature-specs) for your feature tests. For more information on how to get started with feature tests, see [get started with feature tests](#get-started-with-feature-tests) Regression tests should be written for bug fixes to prevent them from recurring in the future. @@ -962,7 +962,7 @@ If an integration test depends on JavaScript to run correctly, you need to make sure the spec is configured to enable JavaScript when the tests are run. If you don't do this, the spec runner displays vague error messages. -To enable a JavaScript driver in an `rspec` test, add `:js` to the +To enable a JavaScript driver in an `RSpec` test, add `:js` to the individual spec or the context block containing multiple specs that need JavaScript enabled: @@ -1242,6 +1242,332 @@ A good guideline to follow: the more complex the component you may want to steer - To just have some kind of test written - To capture highly volatile UI elements without stubbing them (Think of GitLab UI version updates) +## Get started with feature tests + +### What is a feature test + +A [feature test](testing_levels.md#white-box-tests-at-the-system-level-formerly-known-as-system--feature-tests), also known as `white-box testing`, is a test that spawns a browser and has Capybara helpers. This means the test can: + +- Locate an element in the browser. +- Click that element. +- Call the API. + +Feature tests are expensive to run. You should make sure that you **really want** this type of test before running one. + +All of our feature tests are written in `Ruby` but often end up being written by `JavaScript` engineers, as they implement the user-facing feature. So, the following section assumes no prior knowledge of `Ruby` or `Capybara`, and provide a clear guideline on when and how to use these tests. + +### When to use feature tests + +You should use a feature test when the test: + +- Is across multiple components. +- Requires that a user navigate across pages. +- Is submitting a form and observing results elsewhere. +- Would result in a huge number of mocking and stubbing with fake data and components if done as a unit test. + +Feature tests are especially useful when you want to test: + +- That multiple components are working together successfully. +- Complex API interactions. Feature tests interact with the API, so they are slower but do not need any level of mocking or fixtures. + +### When not to use feature tests + +You should use `jest` and `vue-test-utils` unit tests instead of a feature test if you can get the same test results from these methods. Feature tests are quite expensive to run. + +You should use a unit test if: + +- The behavior you are implementing is all in one component. +- You can simulate other components' behavior to trigger the desired effect. +- You can already select UI elements in the virtual DOM to trigger the desired effects. + +Also, if a behavior in your new code needs multiple components to work together, you should consider testing your behavior higher in the component tree. For example, let's say that we have a component called `ParentComponent` with the code: + +```vue + + +``` + +In this example: + +- `ChildComponent1` emits an event. +- `ParentComponent` changes its `internalData` value. +- `ParentComponent` passes the props down to `ChildComponent2`. + +You can use a unit test instead by: + +- From inside the `ParentComponent` unit test file, emitting the expected event from `childComponent1` +- Making sure the prop is passed down to `childComponent2`. + +Then each child component unit tests what happens when the event is emitted and when the prop changes. + +This example also applies at larger scale and with deeper component trees. It is definitely worth using unit tests and avoiding the extra cost of feature tests if you can: + +- Confidently mount child components. +- Emit events or select elements in the virtual DOM. +- Get the test behavior that you want. + +### Where to create your test + +Feature tests live in `spec/features` folder. You should look for existing files that can test the page you are adding a feature to. Within that folder, you can locate your section. For example, if you wanted to add a new feature test for the pipeline page, you would look in `spec/features/projects/pipelines` and see if the test you want to write exists here. + +### How to run a feature test + +1. Make sure that you have a working GDK environment. +1. Start your `gdk` environment with `gdk start` command. +1. In your terminal, run: + + ```shell + bundle exec rspec path/to/file:line_of_my_test + ``` + +You can also prefix this command with `WEBDRIVER_HEADLESS=0` which will run the test by opening an actual browser on your computer that you can see, which is very useful for debugging. + +### How to write a test + +#### Basic file structure + +1. Make all string literals unchangeable + + In all feature tests, the very first line should be: + + ```ruby + # frozen_string_literal: true + ``` + + This is in every `Ruby` file and makes all string literals unchangeable. There are also some performance benefits, but this is beyond the scope of this section. + +1. Import dependencies. + + You should import the modules you need. You will most likely always need to require `spec_helper`: + + ```ruby + require 'spec_helper' + ``` + + Import any other relevant module. + +1. Create a global scope for RSpec to define our tests, just like what we do in jest with the initial describe block. + +Then, you need to create the very first `RSpec` scope. + +```ruby +RSpec.describe 'Pipeline', :js do + ... +end +``` + +What is different though, is that just like everything in Ruby, this is actually a `class`. Which means that right at the top, you can `include` modules that you'd need for your test. For example, you could include the `RoutesHelpers` to navigate more easily. + +```ruby +RSpec.describe 'Pipeline', :js do + include RoutesHelpers + ... +end +``` + +After all of this implementation, we have a file that looks something like this: + +```ruby +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Pipeline', :js do + include RoutesHelpers +end +``` + +#### Seeding data + +Each test is in its own environment and so you must use a factory to seed the required data. To continue on with the pipeline example, let's say that you want a test that takes you to the main pipeline page, which is at the route `/namespace/project/-/pipelines/:id/`. + +Most feature tests at least require you to create a user, because you want to be signed in. You can skip this step if you don't have to be signed in, but as a general rule, you should **always create a user unless you are specifically testing a feature looked at by an anonymous user**. This makes sure that you explicitly set a level of permission that you can edit in the test as needed to change or test a new level of permission as the section changes. To create a user: + +```ruby + let(:user) { create(:user) } +``` + +This creates a variable that holds the newly created user and we can use `create` because we imported the `spec_helper`. + +However, we have not done anything with this user yet because it's just a variable. So, in the `before do` block of the spec, we could sign in with the user so that every spec starts with a signed in user. + +```ruby + let(:user) { create(:user) } + + before do + sign_in(user) + end +``` + +Now that we have a user, we should look at what else we'd need before asserting anything on a pipeline page. If you look at the route `/namespace/project/-/pipelines/:id/` we can determine we need a project and a pipeline. + +So we'd create a project and pipeline, and link them together. Usually in factories, the child element requires its parent as an argument. In this case, a pipeline is a child of a project. So we can create the project first, and then when we create the pipeline, we are pass the project as an argument which "binds" the pipeline to the project. A pipeline is also owned by a user, so we need the user as well. For example, this creates a project and a pipeline: + +```ruby + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id, user: user) } +``` + +In the same spirit, you could then create a job (build) by using the build factory and passing the parent pipeline: + +```ruby + create(:ci_build, pipeline: pipeline, stage_idx: 10, stage: 'publish', name: 'CentOS') +``` + +There are many factories that already exists, so make sure to look at other existing files to see if what you need is available. + +#### Navigation + +You can navigate to a page by using the `visit` method and passing the path as an argument. Rails automatically generates helper paths, so make sure to use these instead of a hardcoded string. They are generated using the route model, so if we want to go to a pipeline, we'd use: + +```ruby + visit project_pipeline_path(project, pipeline) +``` + +Before executing any page interaction when navigating or making asynchronous call through the UI, make sure to use `wait_for_requests` before proceeding with further instructions. + +#### Elements interaction + +There are a lot of different ways to find and interact with elements. For example, you could use the basic `find` method with the `selector` and `text` parameter and then use the `.click` method + +```ruby + find('.gl-tab-nav-item', text: 'Tests').click +``` + +Alternatively, you could use `click_button` with a string of text that is found within the button, which is a more semantically meaningful way of clicking the element. + +```ruby + click_button 'Text inside the button element' +``` + +If you want to follow a link, then there is `click_link`: + +```ruby + click_link 'Text inside the link tag' +``` + +You can use `fill_in` to fill input / form elements. The first argument is the selector, the second is `with:` which is the value to pass in. + +```ruby + fill_in 'current_password', with: '123devops' +``` + +Alternatively, you can use the `find` selector paired with `send_keys` to add keys in a field without removing previous text, or `set` which completely replaces the value of the input element. + +All of these are valid selectors and methods. Pick whichever suits your needs and look around as there are many more useful ones! + +#### Assertions + +To assert anything in a page, you can always access `page` variable, which is automatically defines and actually means the page document. This means you can expect the `page` to have certain components like selectors or content. Here are a few examples: + +```ruby + # Finding an element by ID + expect(page).to have_selector('#js-pipeline-graph') +``` + +```ruby + # Finding by text + expect(page).to have_content('build') +``` + +```ruby + # Finding by `href` value + expect(page).to have_link(pipeline.ref) +``` + +```ruby + # Finding by CSS selector. This is a last resort. + # For example, when you cannot add attributes on the desired element. + expect(page).to have_css('.js-icon-retry') +``` + +```ruby + # Find by data-testid + # Like CSS selector, this is acceptable when there isn't a specific matcher available. + expect(page).to have_selector('[data-testid="pipeline-multi-actions-dropdown"]') +``` + +```ruby + # You can combine any of these selectors with `not_to` instead + expect(page).not_to have_selector('#js-pipeline-graph') +``` + +```ruby + # When a test case has back to back expectations, + # it is recommended to group them using `:aggregate_failures` + it 'shows the issue description and design references', :aggregate_failures do + expect(page).to have_text('The designs I mentioned') + expect(page).to have_link(design_tab_ref) + expect(page).to have_link(design_ref_a) + expect(page).to have_link(design_ref_b) + end +``` + +You can also create a sub-block to look into, to: + +- Scope down where you are making your assertions and reduce the risk of finding another element that was not intended. +- Make sure an element is found within the right boundaries. + +```ruby + page.within('#js-pipeline-graph') do + ... + end +``` + +#### Feature flags + +By default, every feature flag is enabled **regardless of the YAML definition or the flags you've set manually in your GDK**. To test when a feature flag is disabled, you must manually stub the flag, ideally in a `before do` block. + +```ruby + stub_feature_flags(my_feature_flag: false) +``` + +If you are stubbing an `ee` feature flag, then use: + +```ruby + stub_licensed_features(my_feature_flag: false) +``` + +### Debugging + +You can run your spec with the prefix `WEBDRIVER_HEADLESS=0` to open an actual browser. However, the specs goes though the commands quickly and leaves you no time to look around. + +To avoid this problem, you can write `binding.pry` on the line where you want Capybara to stop execution. You are then inside the browser with normal usage. To understand why you cannot find certain elements, you can: + +- Select elements. +- Use the console and network tab. +- Execute selectors inside the browser console. + +Inside the terminal, where capybara is running, you can also execute `next` which goes line by line through the test. This way you can check every single interaction one by one to see what might be causing an issue. + +### Updating ChromeDriver + +On MacOS, if you get a ChromeDriver error, make sure to update it by running + +```shell + brew upgrade chromedriver +``` + --- [Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/index.md b/doc/development/testing_guide/index.md index 753089fa673..ab235f8ba08 100644 --- a/doc/development/testing_guide/index.md +++ b/doc/development/testing_guide/index.md @@ -43,6 +43,11 @@ system tests, parameterized tests etc. Everything you should know about how to write good Frontend tests: Jest, testing promises, stubbing etc. +## [Getting started with Feature tests](frontend_testing.md#get-started-with-feature-tests) + +Need to get started with feature tests? Here are some general guidelines, +tips and tricks to get the most out of white-box testing. + ## [Flaky tests](flaky_tests.md) What are flaky tests, the different kind of flaky tests we encountered, and what diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index 928c28e1fc0..15dba48b8ba 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -343,6 +343,7 @@ namespace_limits: :gitlab_main namespace_package_settings: :gitlab_main namespace_root_storage_statistics: :gitlab_main namespace_ci_cd_settings: :gitlab_main +namespace_commit_emails: :gitlab_main namespace_settings: :gitlab_main namespace_details: :gitlab_main namespaces: :gitlab_main diff --git a/lib/gitlab/usage_data_counters/known_events/work_items.yml b/lib/gitlab/usage_data_counters/known_events/work_items.yml index afa6663de07..d088b6d7e5a 100644 --- a/lib/gitlab/usage_data_counters/known_events/work_items.yml +++ b/lib/gitlab/usage_data_counters/known_events/work_items.yml @@ -19,6 +19,11 @@ redis_slot: users aggregation: weekly feature_flag: track_work_items_activity +- name: users_updating_work_item_milestone + category: work_items + redis_slot: users + aggregation: weekly + feature_flag: track_work_items_activity - name: users_updating_work_item_iteration # The event tracks an EE feature. # It's added here so it can be aggregated into the CE/EE 'OR' aggregate metrics. diff --git a/lib/gitlab/usage_data_counters/work_item_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/work_item_activity_unique_counter.rb index a0fd04596fc..b99c9ebb24f 100644 --- a/lib/gitlab/usage_data_counters/work_item_activity_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/work_item_activity_unique_counter.rb @@ -7,6 +7,7 @@ module Gitlab WORK_ITEM_TITLE_CHANGED = 'users_updating_work_item_title' WORK_ITEM_DATE_CHANGED = 'users_updating_work_item_dates' WORK_ITEM_LABELS_CHANGED = 'users_updating_work_item_labels' + WORK_ITEM_MILESTONE_CHANGED = 'users_updating_work_item_milestone' class << self def track_work_item_created_action(author:) @@ -25,6 +26,10 @@ module Gitlab track_unique_action(WORK_ITEM_LABELS_CHANGED, author) end + def track_work_item_milestone_changed_action(author:) + track_unique_action(WORK_ITEM_MILESTONE_CHANGED, author) + end + private def track_unique_action(action, author) diff --git a/qa/Gemfile b/qa/Gemfile index 7134ad162ba..4eed5aab2bb 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -5,7 +5,7 @@ source 'https://rubygems.org' gem 'gitlab-qa', '~> 8', '>= 8.10.1', require: 'gitlab/qa' gem 'activesupport', '~> 6.1.4.7' # This should stay in sync with the root's Gemfile gem 'allure-rspec', '~> 2.18.0' -gem 'capybara', '~> 3.37.1' +gem 'capybara', '~> 3.38.0' gem 'capybara-screenshot', '~> 1.0.26' gem 'rake', '~> 13', '>= 13.0.6' gem 'rspec', '~> 3.12' diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index eb01db74a08..1b7ccdaa992 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -27,7 +27,7 @@ GEM binding_ninja (0.2.3) builder (3.2.4) byebug (11.1.3) - capybara (3.37.1) + capybara (3.38.0) addressable matrix mini_mime (>= 0.1.3) @@ -302,7 +302,7 @@ DEPENDENCIES activesupport (~> 6.1.4.7) airborne (~> 0.3.7) allure-rspec (~> 2.18.0) - capybara (~> 3.37.1) + capybara (~> 3.38.0) capybara-screenshot (~> 1.0.26) chemlab (~> 0.10) chemlab-library-www-gitlab-com (~> 0.1, >= 0.1.1) diff --git a/qa/qa/support/loglinking.rb b/qa/qa/support/loglinking.rb index af3692c4a8f..f24577ff313 100644 --- a/qa/qa/support/loglinking.rb +++ b/qa/qa/support/loglinking.rb @@ -27,7 +27,7 @@ module QA errors = ["Correlation Id: #{correlation_id}"] errors << "Sentry Url: #{sentry_uri}&query=correlation_id%3A%22#{correlation_id}%22" if sentry_uri - errors << "Kibana Url: #{kibana_uri}app/discover#/?_a=(query:(language:kuery,query:'json.correlation_id%20:%20#{correlation_id}'))&_g=(time:(from:now-24h,to:now))" if kibana_uri + errors << "Kibana Url: #{kibana_uri}app/discover#/?_a=%28query%3A%28language%3Akuery%2Cquery%3A%27json.correlation_id%20%3A%20#{correlation_id}%27%29%29&_g=%28time%3A%28from%3Anow-24h%2Cto%3Anow%29%29" if kibana_uri errors.join("\n") end diff --git a/qa/spec/resource/api_fabricator_spec.rb b/qa/spec/resource/api_fabricator_spec.rb index be6a7841d18..4cb6ef3c9b5 100644 --- a/qa/spec/resource/api_fabricator_spec.rb +++ b/qa/spec/resource/api_fabricator_spec.rb @@ -156,7 +156,7 @@ RSpec.describe QA::Resource::ApiFabricator do Fabrication of FooBarResource using the API failed (400) with `#{raw_post}`. Correlation Id: foobar Sentry Url: https://sentry.gitlab.net/gitlab/staginggitlabcom/?environment=gstg&query=correlation_id%3A%22foobar%22 - Kibana Url: https://nonprod-log.gitlab.net/app/discover#/?_a=(query:(language:kuery,query:'json.correlation_id%20:%20foobar'))&_g=(time:(from:now-24h,to:now)) + Kibana Url: https://nonprod-log.gitlab.net/app/discover#/?_a=%28query%3A%28language%3Akuery%2Cquery%3A%27json.correlation_id%20%3A%20foobar%27%29%29&_g=%28time%3A%28from%3Anow-24h%2Cto%3Anow%29%29 ERROR end end diff --git a/qa/spec/support/loglinking_spec.rb b/qa/spec/support/loglinking_spec.rb index 1e797f5f992..10865068e3d 100644 --- a/qa/spec/support/loglinking_spec.rb +++ b/qa/spec/support/loglinking_spec.rb @@ -29,7 +29,7 @@ RSpec.describe QA::Support::Loglinking do expect(QA::Support::Loglinking.failure_metadata('foo123')).to eql(<<~ERROR.chomp) Correlation Id: foo123 - Kibana Url: https://kibana.address/app/discover#/?_a=(query:(language:kuery,query:'json.correlation_id%20:%20foo123'))&_g=(time:(from:now-24h,to:now)) + Kibana Url: https://kibana.address/app/discover#/?_a=%28query%3A%28language%3Akuery%2Cquery%3A%27json.correlation_id%20%3A%20foo123%27%29%29&_g=%28time%3A%28from%3Anow-24h%2Cto%3Anow%29%29 ERROR end end diff --git a/spec/factories/users/namespace_commit_emails.rb b/spec/factories/users/namespace_commit_emails.rb new file mode 100644 index 00000000000..2f7e89bf766 --- /dev/null +++ b/spec/factories/users/namespace_commit_emails.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :namespace_commit_email, class: 'Users::NamespaceCommitEmail' do + email + user { email.user } + namespace + end +end diff --git a/spec/features/ide/user_opens_merge_request_spec.rb b/spec/features/ide/user_opens_merge_request_spec.rb index 8a95d7c5544..4ffa5212970 100644 --- a/spec/features/ide/user_opens_merge_request_spec.rb +++ b/spec/features/ide/user_opens_merge_request_spec.rb @@ -21,6 +21,6 @@ RSpec.describe 'IDE merge request', :js do wait_for_requests - expect(page).to have_selector('.monaco-diff-editor') + expect(page).not_to have_selector('.monaco-diff-editor') end end diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb index 587556cf5ea..ecf6349e431 100644 --- a/spec/features/projects/branches_spec.rb +++ b/spec/features/projects/branches_spec.rb @@ -7,6 +7,19 @@ RSpec.describe 'Branches' do let_it_be(:project) { create(:project, :public, :repository) } let(:repository) { project.repository } + context 'when logged in as reporter' do + before do + sign_in(user) + project.add_reporter(user) + end + + it 'does not show delete button' do + visit project_branches_path(project) + + expect(page).not_to have_css '.js-delete-branch-button' + end + end + context 'when logged in as developer' do before do sign_in(user) diff --git a/spec/helpers/hooks_helper_spec.rb b/spec/helpers/hooks_helper_spec.rb index 28f6322466f..98a1f77b414 100644 --- a/spec/helpers/hooks_helper_spec.rb +++ b/spec/helpers/hooks_helper_spec.rb @@ -15,7 +15,7 @@ RSpec.describe HooksHelper do it 'returns proper data' do expect(subject).to match( url: project_hook.url, - url_variables: Gitlab::Json.dump([]) + url_variables: "[]" ) end end @@ -26,7 +26,7 @@ RSpec.describe HooksHelper do it 'returns proper data' do expect(subject).to match( url: project_hook.url, - url_variables: Gitlab::Json.dump(['abc']) + url_variables: Gitlab::Json.dump([{ key: 'abc' }]) ) end end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb index e014e7f2c8e..35e5d7f2796 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb @@ -15,6 +15,8 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::WorkItemsActivityAggreg users_creating_work_items users_updating_work_item_title users_updating_work_item_dates + users_updating_work_item_labels + users_updating_work_item_milestone users_updating_work_item_iteration ] } @@ -57,6 +59,7 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::WorkItemsActivityAggreg counter.track_event(:users_updating_work_item_title, values: author1_id, time: event_time) counter.track_event(:users_updating_work_item_dates, values: author1_id, time: event_time) counter.track_event(:users_updating_work_item_labels, values: author1_id, time: event_time) + counter.track_event(:users_updating_work_item_milestone, values: author1_id, time: event_time) end.to not_change { described_class.new(metric_definition).value } expect do diff --git a/spec/lib/gitlab/usage_data_counters/work_item_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/work_item_activity_unique_counter_spec.rb index 2d251017c87..aaf509b6f81 100644 --- a/spec/lib/gitlab/usage_data_counters/work_item_activity_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/work_item_activity_unique_counter_spec.rb @@ -36,4 +36,12 @@ RSpec.describe Gitlab::UsageDataCounters::WorkItemActivityUniqueCounter, :clean_ it_behaves_like 'work item unique counter' end + + describe '.track_work_item_milestone_changed_action' do + subject(:track_event) { described_class.track_work_item_milestone_changed_action(author: user) } + + let(:event_name) { described_class::WORK_ITEM_MILESTONE_CHANGED } + + it_behaves_like 'work item unique counter' + end end diff --git a/spec/models/users/namespace_commit_email_spec.rb b/spec/models/users/namespace_commit_email_spec.rb new file mode 100644 index 00000000000..696dac25f9b --- /dev/null +++ b/spec/models/users/namespace_commit_email_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::NamespaceCommitEmail, type: :model do + subject { build(:namespace_commit_email) } + + describe 'associations' do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:namespace) } + it { is_expected.to belong_to(:email) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:namespace) } + it { is_expected.to validate_presence_of(:email) } + end + + it { is_expected.to be_valid } +end diff --git a/spec/services/clusters/applications/prometheus_update_service_spec.rb b/spec/services/clusters/applications/prometheus_update_service_spec.rb deleted file mode 100644 index 615bfc44045..00000000000 --- a/spec/services/clusters/applications/prometheus_update_service_spec.rb +++ /dev/null @@ -1,111 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Clusters::Applications::PrometheusUpdateService do - describe '#execute' do - let(:project) { create(:project) } - let(:environment) { create(:environment, project: project) } - let(:cluster) { create(:cluster, :provided_by_user, :with_installed_helm, projects: [project]) } - let(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } - let(:empty_alerts_values_update_yaml) { "---\nalertmanager:\n enabled: false\nserverFiles:\n alerts: {}\n" } - let(:helm_client) { instance_double(::Gitlab::Kubernetes::Helm::API) } - - subject(:service) { described_class.new(application, project) } - - context 'when prometheus is a Clusters::Integrations::Prometheus' do - let(:application) { create(:clusters_integrations_prometheus, cluster: cluster) } - - it 'raises NotImplementedError' do - expect { service.execute }.to raise_error(NotImplementedError) - end - end - - context 'when prometheus is externally installed' do - let(:application) { create(:clusters_applications_prometheus, :externally_installed, cluster: cluster) } - - it 'raises NotImplementedError' do - expect { service.execute }.to raise_error(NotImplementedError) - end - end - - context 'when prometheus is a Clusters::Applications::Prometheus' do - let!(:patch_command) { application.patch_command(empty_alerts_values_update_yaml) } - - before do - allow(service).to receive(:patch_command).with(empty_alerts_values_update_yaml).and_return(patch_command) - allow(service).to receive(:helm_api).and_return(helm_client) - end - - context 'when there are no errors' do - before do - expect(helm_client).to receive(:update).with(patch_command) - - allow(::ClusterWaitForAppUpdateWorker) - .to receive(:perform_in) - .and_return(nil) - end - - it 'make the application updating' do - expect(application.cluster).not_to be_nil - - service.execute - - expect(application).to be_updating - end - - it 'updates current config' do - prometheus_config_service = spy(:prometheus_config_service) - - expect(Clusters::Applications::PrometheusConfigService) - .to receive(:new) - .with(project, cluster, application) - .and_return(prometheus_config_service) - - expect(prometheus_config_service) - .to receive(:execute) - .and_return(YAML.safe_load(empty_alerts_values_update_yaml)) - - service.execute - end - - it 'schedules async update status check' do - expect(::ClusterWaitForAppUpdateWorker).to receive(:perform_in).once - - service.execute - end - end - - context 'when k8s cluster communication fails' do - before do - error = ::Kubeclient::HttpError.new(500, 'system failure', nil) - allow(helm_client).to receive(:update).and_raise(error) - end - - it 'make the application update errored' do - service.execute - - expect(application).to be_update_errored - expect(application.status_reason).to match(/kubernetes error:/i) - end - end - - context 'when application cannot be persisted' do - let(:application) { build(:clusters_applications_prometheus, :installed) } - - before do - allow(application).to receive(:make_updating!).once - .and_raise(ActiveRecord::RecordInvalid.new(application)) - end - - it 'make the application update errored' do - expect(helm_client).not_to receive(:update) - - service.execute - - expect(application).to be_update_errored - end - end - end - end -end diff --git a/spec/services/resource_events/change_milestone_service_spec.rb b/spec/services/resource_events/change_milestone_service_spec.rb index ed234376381..425d5b19907 100644 --- a/spec/services/resource_events/change_milestone_service_spec.rb +++ b/spec/services/resource_events/change_milestone_service_spec.rb @@ -14,4 +14,35 @@ RSpec.describe ResourceEvents::ChangeMilestoneService do let_it_be(:resource) { create(issuable) } # rubocop:disable Rails/SaveBang end end + + describe 'events tracking' do + let_it_be(:user) { create(:user) } + + let(:resource) { create(resource_type, milestone: timebox, project: timebox.project) } + + subject(:service_instance) { described_class.new(resource, user, old_milestone: nil) } + + context 'when the resource is a work item' do + let(:resource_type) { :work_item } + + it 'tracks work item usage data counters' do + expect(Gitlab::UsageDataCounters::WorkItemActivityUniqueCounter) + .to receive(:track_work_item_milestone_changed_action) + .with(author: user) + + service_instance.execute + end + end + + context 'when the resource is not a work item' do + let(:resource_type) { :issue } + + it 'does not track work item usage data counters' do + expect(Gitlab::UsageDataCounters::WorkItemActivityUniqueCounter) + .not_to receive(:track_work_item_milestone_changed_action) + + service_instance.execute + end + end + end end diff --git a/spec/workers/cluster_update_app_worker_spec.rb b/spec/workers/cluster_update_app_worker_spec.rb deleted file mode 100644 index 8f61ee17162..00000000000 --- a/spec/workers/cluster_update_app_worker_spec.rb +++ /dev/null @@ -1,112 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ClusterUpdateAppWorker do - include ExclusiveLeaseHelpers - - let_it_be(:project) { create(:project) } - - let(:prometheus_update_service) { spy } - - subject { described_class.new } - - around do |example| - freeze_time { example.run } - end - - before do - allow(::Clusters::Applications::PrometheusUpdateService).to receive(:new).and_return(prometheus_update_service) - end - - describe '#perform' do - context 'when the application last_update_started_at is higher than the time the job was scheduled in' do - it 'does nothing' do - application = create(:clusters_applications_prometheus, :updated, last_update_started_at: Time.current) - - expect(prometheus_update_service).not_to receive(:execute) - - expect(subject.perform(application.name, application.id, project.id, Time.current - 5.minutes)).to be_nil - end - end - - context 'when another worker is already running' do - it 'returns nil' do - application = create(:clusters_applications_prometheus, :updating) - - expect(subject.perform(application.name, application.id, project.id, Time.current)).to be_nil - end - end - - it 'executes PrometheusUpdateService' do - application = create(:clusters_applications_prometheus, :installed) - - expect(prometheus_update_service).to receive(:execute) - - subject.perform(application.name, application.id, project.id, Time.current) - end - - context 'application is externally installed' do - it 'does not execute PrometheusUpdateService' do - application = create(:clusters_applications_prometheus, :externally_installed) - - expect(prometheus_update_service).not_to receive(:execute) - - subject.perform(application.name, application.id, project.id, Time.current) - end - end - - context 'with exclusive lease' do - let_it_be(:user) { create(:user) } - - let(:application) { create(:clusters_applications_prometheus, :installed) } - let(:lease_key) { "#{described_class.name.underscore}-#{application.id}" } - - before do - # update_highest_role uses exclusive key too: - allow(Gitlab::ExclusiveLease).to receive(:new).and_call_original - stub_exclusive_lease_taken(lease_key) - end - - it 'does not allow same app to be updated concurrently by same project' do - expect(Clusters::Applications::PrometheusUpdateService).not_to receive(:new) - - subject.perform(application.name, application.id, project.id, Time.current) - end - - it 'does not allow same app to be updated concurrently by different project', :aggregate_failures do - project1 = create(:project, namespace: create(:namespace, owner: user)) - - expect(Clusters::Applications::PrometheusUpdateService).not_to receive(:new) - - subject.perform(application.name, application.id, project1.id, Time.current) - end - - it 'allows different app to be updated concurrently by same project' do - application2 = create(:clusters_applications_prometheus, :installed) - lease_key2 = "#{described_class.name.underscore}-#{application2.id}" - - stub_exclusive_lease(lease_key2) - - expect(Clusters::Applications::PrometheusUpdateService).to receive(:new) - .with(application2, project) - - subject.perform(application2.name, application2.id, project.id, Time.current) - end - - it 'allows different app to be updated by different project', :aggregate_failures do - application2 = create(:clusters_applications_prometheus, :installed) - lease_key2 = "#{described_class.name.underscore}-#{application2.id}" - - project2 = create(:project, namespace: create(:namespace, owner: user)) - - stub_exclusive_lease(lease_key2) - - expect(Clusters::Applications::PrometheusUpdateService).to receive(:new) - .with(application2, project2) - - subject.perform(application2.name, application2.id, project2.id, Time.current) - end - end - end -end