diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-06 06:11:21 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-06 06:11:21 +0300 |
commit | b8fbfa9c705435090b9342efd47f2b27becb7819 (patch) | |
tree | 1fed7825d1deab50bbc48172b412702a7b371749 | |
parent | 102640e087fa826f069a7da3f9871b4b86957285 (diff) |
Add latest changes from gitlab-org/gitlab@master
48 files changed, 334 insertions, 220 deletions
diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index 99911f6e9a7..cfecc4756b9 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -1278,7 +1278,6 @@ Layout/ArgumentAlignment: - 'ee/spec/services/vulnerabilities/user_notes_count_service_spec.rb' - 'ee/spec/services/vulnerability_feedback/create_service_spec.rb' - 'ee/spec/services/vulnerability_merge_request_links/create_service_spec.rb' - - 'ee/spec/tasks/gitlab/elastic_rake_spec.rb' - 'lib/api/access_requests.rb' - 'lib/api/admin/plan_limits.rb' - 'lib/api/alert_management_alerts.rb' diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index ed2245f39e0..72f43c7779e 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -1305,7 +1305,6 @@ Layout/LineLength: - 'ee/lib/system_check/geo/geo_database_configured_check.rb' - 'ee/lib/tasks/geo.rake' - 'ee/lib/tasks/geo/git.rake' - - 'ee/lib/tasks/gitlab/elastic.rake' - 'ee/lib/tasks/gitlab/elastic/test.rake' - 'ee/lib/tasks/gitlab/seed/metrics.rake' - 'ee/lib/world.rb' diff --git a/.rubocop_todo/lint/unused_block_argument.yml b/.rubocop_todo/lint/unused_block_argument.yml index 0b4120a152c..01510a8443d 100644 --- a/.rubocop_todo/lint/unused_block_argument.yml +++ b/.rubocop_todo/lint/unused_block_argument.yml @@ -115,7 +115,6 @@ Lint/UnusedBlockArgument: - 'ee/lib/gitlab/insights/reducers/count_per_label_reducer.rb' - 'ee/lib/gitlab/proxy.rb' - 'ee/lib/tasks/contracts/merge_requests.rake' - - 'ee/lib/tasks/gitlab/elastic.rake' - 'ee/lib/tasks/gitlab/indexer.rake' - 'ee/lib/tasks/gitlab/seed/insights.rake' - 'ee/spec/elastic/migrate/20220118150500_delete_orphaned_commits_spec.rb' diff --git a/.rubocop_todo/lint/unused_method_argument.yml b/.rubocop_todo/lint/unused_method_argument.yml index d1d626b68e0..086882b0f1b 100644 --- a/.rubocop_todo/lint/unused_method_argument.yml +++ b/.rubocop_todo/lint/unused_method_argument.yml @@ -316,7 +316,6 @@ Lint/UnusedMethodArgument: - 'ee/lib/gitlab/package_metadata/connector/gcp.rb' - 'ee/lib/gitlab/package_metadata/connector/offline.rb' - 'ee/lib/gitlab/zoekt/search_results.rb' - - 'ee/lib/tasks/gitlab/elastic.rake' - 'ee/spec/features/boards/swimlanes/epics_swimlanes_filtering_spec.rb' - 'ee/spec/features/groups/group_roadmap_spec.rb' - 'ee/spec/graphql/ee/resolvers/namespace_projects_resolver_spec.rb' diff --git a/.rubocop_todo/naming/heredoc_delimiter_naming.yml b/.rubocop_todo/naming/heredoc_delimiter_naming.yml index a8265e86ca0..68129f910d1 100644 --- a/.rubocop_todo/naming/heredoc_delimiter_naming.yml +++ b/.rubocop_todo/naming/heredoc_delimiter_naming.yml @@ -25,7 +25,6 @@ Naming/HeredocDelimiterNaming: - 'ee/spec/services/security/security_orchestration_policies/create_pipeline_service_spec.rb' - 'ee/spec/services/security/security_orchestration_policies/policy_commit_service_spec.rb' - 'ee/spec/support/helpers/ee/ldap_helpers.rb' - - 'ee/spec/tasks/gitlab/elastic_rake_spec.rb' - 'lib/api/metadata.rb' - 'lib/backup/helper.rb' - 'lib/feature/shared.rb' diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml index ba82a9f7a1e..c1751ebed4c 100644 --- a/.rubocop_todo/rspec/context_wording.yml +++ b/.rubocop_todo/rspec/context_wording.yml @@ -872,7 +872,6 @@ RSpec/ContextWording: - 'ee/spec/support/shared_examples/services/search_notes_shared_examples.rb' - 'ee/spec/support/shared_examples/services/search_service_shared_examples.rb' - 'ee/spec/support/shared_examples/services/update_issuable_health_status_shared_examples.rb' - - 'ee/spec/tasks/gitlab/elastic_rake_spec.rb' - 'ee/spec/tasks/gitlab/license_rake_spec.rb' - 'ee/spec/views/admin/application_settings/_elasticsearch_form.html.haml_spec.rb' - 'ee/spec/views/admin/users/show.html.haml_spec.rb' diff --git a/.rubocop_todo/search/namespaced_class.yml b/.rubocop_todo/search/namespaced_class.yml index 452b9388043..8074e90fc53 100644 --- a/.rubocop_todo/search/namespaced_class.yml +++ b/.rubocop_todo/search/namespaced_class.yml @@ -93,6 +93,9 @@ Search/NamespacedClass: - 'ee/lib/elastic/latest/config.rb' - 'ee/lib/elastic/latest/custom_language_analyzers.rb' - 'ee/lib/elastic/latest/document_should_be_deleted_from_index_error.rb' + - 'ee/lib/elastic/latest/epic_class_proxy.rb' + - 'ee/lib/elastic/latest/epic_config.rb' + - 'ee/lib/elastic/latest/epic_instance_proxy.rb' - 'ee/lib/elastic/latest/git_class_proxy.rb' - 'ee/lib/elastic/latest/git_instance_proxy.rb' - 'ee/lib/elastic/latest/issue_class_proxy.rb' @@ -131,6 +134,8 @@ Search/NamespacedClass: - 'ee/lib/elastic/v12p1/application_class_proxy.rb' - 'ee/lib/elastic/v12p1/application_instance_proxy.rb' - 'ee/lib/elastic/v12p1/config.rb' + - 'ee/lib/elastic/v12p1/epic_class_proxy.rb' + - 'ee/lib/elastic/v12p1/epic_instance_proxy.rb' - 'ee/lib/elastic/v12p1/issue_class_proxy.rb' - 'ee/lib/elastic/v12p1/issue_instance_proxy.rb' - 'ee/lib/elastic/v12p1/merge_request_class_proxy.rb' diff --git a/.rubocop_todo/style/format_string.yml b/.rubocop_todo/style/format_string.yml index 9d202dcd7fb..d03718c0d51 100644 --- a/.rubocop_todo/style/format_string.yml +++ b/.rubocop_todo/style/format_string.yml @@ -237,7 +237,6 @@ Style/FormatString: - 'ee/lib/gitlab/licenses/submit_license_usage_data_banner.rb' - 'ee/lib/gitlab/manual_quarterly_co_term_banner.rb' - 'ee/lib/gitlab/vulnerabilities/container_scanning_vulnerability.rb' - - 'ee/lib/tasks/gitlab/elastic.rake' - 'ee/spec/controllers/admin/licenses_controller_spec.rb' - 'ee/spec/controllers/groups/security/policies_controller_spec.rb' - 'ee/spec/features/admin/admin_settings_spec.rb' diff --git a/.rubocop_todo/style/string_concatenation.yml b/.rubocop_todo/style/string_concatenation.yml index 34e4549a987..a83c0f4d943 100644 --- a/.rubocop_todo/style/string_concatenation.yml +++ b/.rubocop_todo/style/string_concatenation.yml @@ -54,7 +54,6 @@ Style/StringConcatenation: - 'ee/lib/gitlab/elastic/search_results.rb' - 'ee/lib/gitlab/geo/git_ssh_proxy.rb' - 'ee/lib/omni_auth/strategies/kerberos.rb' - - 'ee/lib/tasks/gitlab/elastic.rake' - 'ee/lib/tasks/gitlab/license.rake' - 'ee/spec/features/boards/boards_spec.rb' - 'ee/spec/features/projects/pipelines/pipeline_spec.rb' diff --git a/.rubocop_todo/style/string_literals_in_interpolation.yml b/.rubocop_todo/style/string_literals_in_interpolation.yml index ceb6a922270..30a649cb4fa 100644 --- a/.rubocop_todo/style/string_literals_in_interpolation.yml +++ b/.rubocop_todo/style/string_literals_in_interpolation.yml @@ -18,7 +18,6 @@ Style/StringLiteralsInInterpolation: - 'ee/app/models/license.rb' - 'ee/app/services/epics/tree_reorder_service.rb' - 'ee/lib/ee/api/helpers/issues_helpers.rb' - - 'ee/lib/tasks/gitlab/elastic.rake' - 'ee/spec/features/admin/admin_settings_spec.rb' - 'ee/spec/features/subscriptions/expiring_subscription_message_spec.rb' - 'ee/spec/lib/gitlab/expiring_subscription_message_spec.rb' diff --git a/GITLAB_KAS_VERSION b/GITLAB_KAS_VERSION index e8233db7991..02ec429db93 100644 --- a/GITLAB_KAS_VERSION +++ b/GITLAB_KAS_VERSION @@ -1 +1 @@ -v16.1.3 +v16.2.0-rc1 diff --git a/app/assets/javascripts/pipelines/components/pipeline_details_header.vue b/app/assets/javascripts/pipelines/components/pipeline_details_header.vue index fbd0fb7afbe..88534095f67 100644 --- a/app/assets/javascripts/pipelines/components/pipeline_details_header.vue +++ b/app/assets/javascripts/pipelines/components/pipeline_details_header.vue @@ -17,6 +17,7 @@ import { __, s__, sprintf, formatNumber } from '~/locale'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import CiBadgeLink from '~/vue_shared/components/ci_badge_link.vue'; +import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import SafeHtml from '~/vue_shared/directives/safe_html'; import { LOAD_FAILURE, @@ -30,7 +31,6 @@ import cancelPipelineMutation from '../graphql/mutations/cancel_pipeline.mutatio import deletePipelineMutation from '../graphql/mutations/delete_pipeline.mutation.graphql'; import retryPipelineMutation from '../graphql/mutations/retry_pipeline.mutation.graphql'; import getPipelineQuery from '../graphql/queries/get_pipeline_header_data.query.graphql'; -import TimeAgo from './pipelines_list/time_ago.vue'; import { getQueryHeaders } from './graph/utils'; const DELETE_MODAL_ID = 'pipeline-delete-modal'; @@ -54,7 +54,7 @@ export default { GlLoadingIcon, GlModal, GlSprintf, - TimeAgo, + TimeAgoTooltip, }, directives: { GlModal: GlModalDirective, @@ -90,6 +90,8 @@ export default { cancelPipelineText: __('Cancel pipeline'), deletePipelineText: __('Delete'), clipboardTooltip: __('Copy commit SHA'), + createdText: s__('Pipelines|created'), + finishedText: s__('Pipelines|finished'), }, errorTexts: { [LOAD_FAILURE]: __('We are currently unable to fetch data for the pipeline header.'), @@ -441,14 +443,14 @@ export default { :title="$options.i18n.clipboardTooltip" size="small" /> - <time-ago - v-if="isFinished" - :pipeline="pipeline" - class="gl-display-inline gl-mb-0" - :display-calendar-icon="false" - font-size="gl-font-md" - data-testid="pipeline-time-ago" - /> + <span v-if="inProgress" data-testid="pipeline-created-time-ago"> + {{ $options.i18n.createdText }} + <time-ago-tooltip :time="pipeline.createdAt" /> + </span> + <span v-if="isFinished" data-testid="pipeline-finished-time-ago"> + {{ $options.i18n.finishedText }} + <time-ago-tooltip :time="pipeline.finishedAt" /> + </span> </div> </div> <div v-safe-html="refText" class="gl-mb-2" data-testid="pipeline-ref-text"></div> diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/time_ago.vue b/app/assets/javascripts/pipelines/components/pipelines_list/time_ago.vue index bdecbb88a58..70343544638 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_list/time_ago.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_list/time_ago.vue @@ -14,11 +14,6 @@ export default { type: Object, required: true, }, - displayCalendarIcon: { - type: Boolean, - required: false, - default: true, - }, fontSize: { type: String, required: false, @@ -50,13 +45,7 @@ export default { </p> <p v-if="finishedTime" class="finished-at gl-display-inline-flex gl-align-items-center"> - <gl-icon - v-if="displayCalendarIcon" - name="calendar" - class="gl-mr-2" - :size="12" - data-testid="calendar-icon" - /> + <gl-icon name="calendar" class="gl-mr-2" :size="12" data-testid="calendar-icon" /> <time v-gl-tooltip diff --git a/app/models/namespaces/traversal/linear.rb b/app/models/namespaces/traversal/linear.rb index 9006f104c64..469483931f0 100644 --- a/app/models/namespaces/traversal/linear.rb +++ b/app/models/namespaces/traversal/linear.rb @@ -96,13 +96,6 @@ module Namespaces traversal_ids.present? end - def use_traversal_ids_for_self_and_hierarchy? - return false unless use_traversal_ids? - return false unless Feature.enabled?(:use_traversal_ids_for_self_and_hierarchy, root_ancestor) - - traversal_ids.present? - end - def use_traversal_ids_for_ancestors? return false unless use_traversal_ids? return false unless Feature.enabled?(:use_traversal_ids_for_ancestors, root_ancestor) @@ -150,7 +143,7 @@ module Namespaces end def self_and_hierarchy - return super unless use_traversal_ids_for_self_and_hierarchy? + return super unless use_traversal_ids? self_and_descendants.or(ancestors) end diff --git a/app/services/users/ban_service.rb b/app/services/users/ban_service.rb index 5ed31cdb778..20c34b15f15 100644 --- a/app/services/users/ban_service.rb +++ b/app/services/users/ban_service.rb @@ -2,6 +2,8 @@ module Users class BanService < BannedUserBaseService + extend ::Gitlab::Utils::Override + private def update_user(user) @@ -15,6 +17,11 @@ module Users def action :ban end + + override :track_event + def track_event(user) + experiment(:phone_verification_for_low_risk_users, user: user).track(:banned) + end end end diff --git a/app/services/users/banned_user_base_service.rb b/app/services/users/banned_user_base_service.rb index 74c10581a6e..cec351904a9 100644 --- a/app/services/users/banned_user_base_service.rb +++ b/app/services/users/banned_user_base_service.rb @@ -12,6 +12,7 @@ module Users if update_user(user) log_event(user) + track_event(user) success else messages = user.errors.full_messages @@ -23,6 +24,9 @@ module Users attr_reader :current_user + # Overridden in Users::BanService + def track_event(_); end + def state_error(user) error(_("You cannot %{action} %{state} users." % { action: action.to_s, state: user.state }), :forbidden) end diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml index bdcc7dda644..92a664e1ca8 100644 --- a/app/views/admin/hooks/_form.html.haml +++ b/app/views/admin/hooks/_form.html.haml @@ -6,10 +6,10 @@ %p.form-text.text-muted= _('URL must be percent-encoded if necessary.') .form-group = form.label :token, _('Secret token'), class: 'label-bold' - = form.text_field :token, class: 'form-control gl-form-input gl-max-w-48' + = form.password_field :token, value: hook.masked_token, autocomplete: 'new-password', class: 'form-control gl-form-input gl-max-w-48' %p.form-text.text-muted= _('Use this token to validate received payloads.') .form-group - = form.label :url, _('Trigger'), class: 'label-bold' + = form.label :url, _('Trigger'), class: 'label-bold gl-mb-0' .form-text.text-secondary.gl-mb-5= _('System hooks are triggered on sets of events like creating a project or adding an SSH key. You can also enable extra triggers, such as push events.') %fieldset.form-group = form.gitlab_ui_checkbox_component :repository_update_events, _('Repository update events'), diff --git a/app/views/admin/hooks/edit.html.haml b/app/views/admin/hooks/edit.html.haml index 14d37b77a41..b0e45f0a49f 100644 --- a/app/views/admin/hooks/edit.html.haml +++ b/app/views/admin/hooks/edit.html.haml @@ -3,17 +3,16 @@ = render 'shared/web_hooks/hook_errors', hook: @hook -.row.gl-mt-3 - .col-lg-3 - = render 'shared/web_hooks/title_and_docs', hook: @hook +.gl-mt-5 + = render 'shared/web_hooks/title_and_docs', hook: @hook - .col-lg-9.gl-mb-3 - = gitlab_ui_form_for @hook, as: :hook, url: admin_hook_path do |f| - = render partial: 'form', locals: { form: f, hook: @hook } - .form-actions - %span>= f.submit _('Save changes'), class: 'gl-mr-3', pajamas_button: true - = render 'shared/web_hooks/test_button', hook: @hook - = link_to _('Delete'), admin_hook_path(@hook), method: :delete, class: 'btn gl-button btn-danger float-right', aria: { label: s_('Webhooks|Delete webhook') }, data: { confirm: s_('Webhooks|Are you sure you want to delete this webhook?'), confirm_btn_variant: 'danger' } + = gitlab_ui_form_for @hook, as: :hook, url: admin_hook_path do |f| + = render partial: 'form', locals: { form: f, hook: @hook } + + %div + = f.submit _('Save changes'), pajamas_button: true, class: 'gl-sm-mr-3' + = render 'shared/web_hooks/test_button', hook: @hook + = link_to _('Delete'), admin_hook_path(@hook), method: :delete, class: 'btn gl-button btn-danger gl-float-right', aria: { label: s_('Webhooks|Delete webhook') }, data: { confirm: s_('Webhooks|Are you sure you want to delete this webhook?'), confirm_btn_variant: 'danger' } %hr diff --git a/app/views/projects/hooks/edit.html.haml b/app/views/projects/hooks/edit.html.haml index b553249c4b8..b410b6c07e1 100644 --- a/app/views/projects/hooks/edit.html.haml +++ b/app/views/projects/hooks/edit.html.haml @@ -3,17 +3,16 @@ = render 'shared/web_hooks/hook_errors', hook: @hook -.row.gl-mt-3 - .col-lg-3 - = render 'shared/web_hooks/title_and_docs', hook: @hook +.gl-mt-5 + = render 'shared/web_hooks/title_and_docs', hook: @hook - .col-lg-9.gl-mb-3 - = gitlab_ui_form_for [@project, @hook], as: :hook, url: project_hook_path(@project, @hook), html: { class: 'js-webhook-form' } do |f| - = render partial: 'shared/web_hooks/form', locals: { form: f, hook: @hook } + = gitlab_ui_form_for [@project, @hook], as: :hook, url: project_hook_path(@project, @hook), html: { class: 'js-webhook-form' } do |f| + = render partial: 'shared/web_hooks/form', locals: { form: f, hook: @hook } - = f.submit _('Save changes'), pajamas_button: true + %div + = f.submit _('Save changes'), pajamas_button: true, class: 'gl-sm-mr-3' = render 'shared/web_hooks/test_button', hook: @hook - = link_to _('Delete'), project_hook_path(@project, @hook), method: :delete, class: 'btn gl-button btn-danger float-right', aria: { label: s_('Webhooks|Delete webhook') }, data: { confirm: s_('Webhooks|Are you sure you want to delete this project hook?'), confirm_btn_variant: 'danger' } + = link_to _('Delete'), project_hook_path(@project, @hook), method: :delete, class: 'btn gl-button btn-danger gl-float-right', aria: { label: s_('Webhooks|Delete webhook') }, data: { confirm: s_('Webhooks|Are you sure you want to delete this project hook?'), confirm_btn_variant: 'danger' } %hr diff --git a/app/views/shared/hook_logs/_index.html.haml b/app/views/shared/hook_logs/_index.html.haml index 7dab14b95c1..d4ed085be04 100644 --- a/app/views/shared/hook_logs/_index.html.haml +++ b/app/views/shared/hook_logs/_index.html.haml @@ -2,10 +2,7 @@ - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: docs_link_url } - link_end = '</a>'.html_safe -.row.gl-mt-3.gl-mb-3 - .col-lg-3 - %h4.gl-mt-0 - = _('Recent events') - %p= _('GitLab events trigger webhooks. Use the request details of a webhook to help troubleshoot problems. %{link_start}How do I troubleshoot?%{link_end}').html_safe % { link_start: link_start, link_end: link_end } - .col-lg-9 - = render partial: 'shared/hook_logs/recent_deliveries_table', locals: { hook: hook, hook_logs: hook_logs } +.gl-mt-5 + %h4.gl-my-0= _('Recent events') + %p.gl-text-secondary= _('GitLab events trigger webhooks. Use the request details of a webhook to help troubleshoot problems. %{link_start}How do I troubleshoot?%{link_end}').html_safe % { link_start: link_start, link_end: link_end } + = render partial: 'shared/hook_logs/recent_deliveries_table', locals: { hook: hook, hook_logs: hook_logs } diff --git a/config/feature_flags/development/use_traversal_ids_for_self_and_hierarchy.yml b/config/feature_flags/development/use_traversal_ids_for_self_and_hierarchy.yml deleted file mode 100644 index e1f1ec0df35..00000000000 --- a/config/feature_flags/development/use_traversal_ids_for_self_and_hierarchy.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: use_traversal_ids_for_self_and_hierarchy -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76814 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348527 -milestone: '14.7' -type: development -group: group::tenant scale -default_enabled: true diff --git a/config/feature_flags/ops/search_index_curation_epics.yml b/config/feature_flags/ops/search_index_curation_epics.yml new file mode 100644 index 00000000000..73eab662cb6 --- /dev/null +++ b/config/feature_flags/ops/search_index_curation_epics.yml @@ -0,0 +1,8 @@ +--- +name: search_index_curation_epics +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121635 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/413605 +milestone: '16.2' +type: ops +group: group::global search +default_enabled: false diff --git a/config/metrics/counts_28d/20210216181323_g_project_management_issue_created_monthly.yml b/config/metrics/counts_28d/20210216181323_g_project_management_issue_created_monthly.yml index dd1fa78ceef..cd5f4142e80 100644 --- a/config/metrics/counts_28d/20210216181323_g_project_management_issue_created_monthly.yml +++ b/config/metrics/counts_28d/20210216181323_g_project_management_issue_created_monthly.yml @@ -15,7 +15,7 @@ options: - g_project_management_issue_created events: - name: g_project_management_issue_created - unique: user_id + unique: user.id distribution: - ce - ee diff --git a/config/metrics/counts_7d/20210216175130_i_code_review_user_create_mr_weekly.yml b/config/metrics/counts_7d/20210216175130_i_code_review_user_create_mr_weekly.yml index f5c8412de3e..0159d8c6507 100644 --- a/config/metrics/counts_7d/20210216175130_i_code_review_user_create_mr_weekly.yml +++ b/config/metrics/counts_7d/20210216175130_i_code_review_user_create_mr_weekly.yml @@ -15,7 +15,7 @@ options: - i_code_review_user_create_mr events: - name: i_code_review_user_create_mr - unique: user_id + unique: user.id distribution: - ce - ee diff --git a/config/metrics/counts_7d/20210216181321_g_project_management_issue_created_weekly.yml b/config/metrics/counts_7d/20210216181321_g_project_management_issue_created_weekly.yml index 79911ae8cdc..ca0194db21b 100644 --- a/config/metrics/counts_7d/20210216181321_g_project_management_issue_created_weekly.yml +++ b/config/metrics/counts_7d/20210216181321_g_project_management_issue_created_weekly.yml @@ -15,7 +15,7 @@ options: - g_project_management_issue_created events: - name: g_project_management_issue_created - unique: user_id + unique: user.id distribution: - ce - ee diff --git a/config/metrics/schema.json b/config/metrics/schema.json index d3afc779e14..56fd617932c 100644 --- a/config/metrics/schema.json +++ b/config/metrics/schema.json @@ -95,6 +95,29 @@ "options": { "type": "object" }, + "events": { + "type": "array", + "items": { + "type": "object", + "required": [ + "name", + "unique" + ], + "properties": { + "name": { + "type": "string" + }, + "unique": { + "type": "string", + "enum": [ + "user.id", + "project.id", + "namespace.id" + ] + } + } + } + }, "time_frame": { "type": "string", "enum": [ diff --git a/db/post_migrate/20230704042302_prepare_removal_index_deployments_on_project_id_sha.rb b/db/post_migrate/20230704042302_prepare_removal_index_deployments_on_project_id_sha.rb new file mode 100644 index 00000000000..77ddd909215 --- /dev/null +++ b/db/post_migrate/20230704042302_prepare_removal_index_deployments_on_project_id_sha.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class PrepareRemovalIndexDeploymentsOnProjectIdSha < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'index_deployments_on_project_id_sha' + + # TODO: Index to be destroyed synchronously in https://gitlab.com/gitlab-org/gitlab/-/issues/402512 + def up + prepare_async_index_removal :deployments, %i[project_id sha], name: INDEX_NAME + end + + def down + unprepare_async_index :deployments, %i[project_id sha], name: INDEX_NAME + end +end diff --git a/db/schema_migrations/20230704042302 b/db/schema_migrations/20230704042302 new file mode 100644 index 00000000000..0a016cd7bb2 --- /dev/null +++ b/db/schema_migrations/20230704042302 @@ -0,0 +1 @@ +d4c93417aef4587ba892f81b0339c5213cd6b5270478edcb1378138fd74e2787
\ No newline at end of file diff --git a/doc/user/application_security/vulnerabilities/index.md b/doc/user/application_security/vulnerabilities/index.md index 39d8e28e564..202f6096e45 100644 --- a/doc/user/application_security/vulnerabilities/index.md +++ b/doc/user/application_security/vulnerabilities/index.md @@ -200,7 +200,9 @@ To manually apply the patch that GitLab generated for a vulnerability: NOTE: Security training is not available in an offline environment because it uses content from -third-party vendors. +third-party vendors. Some third-party training vendors may require you to sign up for a _free_ account. Sign up for an account by going to +any of [Secure Code Warrior](https://www.securecodewarrior.com/), [Kontra](https://application.security/), or [SecureFlag](https://www.secureflag.com/). +GitLab does not send any user information to these third-party vendors; we do send the CWE or OWASP identifier and the language name of the file extension. Security training helps your developers learn how to fix vulnerabilities. Developers can view security training from selected educational providers, relevant to the detected vulnerability. @@ -211,6 +213,8 @@ To enable security training for vulnerabilities in your project: 1. On the tab bar, select **Vulnerability Management**. 1. To enable a security training provider, turn on the toggle. +Each integration submits the Vulnerability identifier, for example CWE or OWASP, and the language to the security training vendor. The resulting link to the vendor training is what appears in a GitLab Vulnerability. + ## View security training for a vulnerability > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/6176) in GitLab 14.9. @@ -218,8 +222,8 @@ To enable security training for vulnerabilities in your project: The vulnerability page may include a training link relevant to the detected vulnerability if security training is enabled. The availability of training depends on whether the enabled training vendor has content matching the particular vulnerability. Training content is requested based on the [vulnerability identifiers](../../../development/integrations/secure.md#identifiers). -The identifier given to a vulnerability varies from one vulnerability to the next. The available training -content varies between vendors. This means some vulnerabilities do not display training content. +The identifier given to a vulnerability varies from one vulnerability to the next and the available training +content varies between vendors. Some vulnerabilities do not display training content. Vulnerabilities with a CWE are most likely to return a training result. To view the security training for a vulnerability: diff --git a/lib/generators/gitlab/analytics/internal_events_generator.rb b/lib/generators/gitlab/analytics/internal_events_generator.rb index a85cdd352d5..05b47e1d501 100644 --- a/lib/generators/gitlab/analytics/internal_events_generator.rb +++ b/lib/generators/gitlab/analytics/internal_events_generator.rb @@ -26,8 +26,8 @@ module Gitlab end end.freeze - NEGATIVE_ANSWERS = %w[no n].freeze - POSITIVE_ANSWERS = %w[yes y].freeze + NEGATIVE_ANSWERS = %w[no n No NO N].freeze + POSITIVE_ANSWERS = %w[yes y Yes YES Y].freeze TOP_LEVEL_DIR = 'config' TOP_LEVEL_DIR_EE = 'ee' DESCRIPTION_MIN_LENGTH = 50 @@ -81,7 +81,7 @@ module Gitlab type: :string, optional: false, desc: 'Name of the event that this metric counts' - class_option :unique_on, + class_option :unique, type: :string, optional: false, desc: 'Name of the event property that this metric counts' @@ -185,7 +185,7 @@ module Gitlab end def key_path(time_frame) - "count_distinct_#{options[:unique_on]}_from_#{event}_#{time_frame}" + "count_distinct_#{options[:unique]}_from_#{event}_#{time_frame}" end def metric_file_path(time_frame) @@ -204,7 +204,7 @@ module Gitlab validate_tiers! - %i[unique_on event mr section stage group].each do |option| + %i[unique event mr section stage group].each do |option| raise "The option: --#{option} is missing" unless options.key? option end diff --git a/lib/gitlab/database/migrations/test_batched_background_runner.rb b/lib/gitlab/database/migrations/test_batched_background_runner.rb index af853c933ba..c5e0b361df5 100644 --- a/lib/gitlab/database/migrations/test_batched_background_runner.rb +++ b/lib/gitlab/database/migrations/test_batched_background_runner.rb @@ -57,6 +57,20 @@ module Gitlab job_arguments: migration.job_arguments ) + # If no rows match, the next_bounds are nil. + # This will only happen if there are zero rows to match from the current sampling point to the end + # of the table + # Simulate the approach in the actual background migration worker by not sampling a batch + # from this range. + # (The actual worker would finish the migration, but we may find batches that can be sampled elsewhere + # in the table) + if next_bounds.nil? + # If the migration has no work to do across the entire table, sampling can get stuck + # in a loop if we don't mark the attempted batches as completed + completed_batches << (batch_start..(batch_start + migration.batch_size)) + next + end + batch_min, batch_max = next_bounds job = migration.create_batched_job!(batch_min, batch_max) @@ -65,7 +79,7 @@ module Gitlab job end - end + end.reject(&:nil?) # Remove skipped batches from the lazy list of batches to test job_class_name = migration.job_class_name diff --git a/lib/gitlab/internal_events.rb b/lib/gitlab/internal_events.rb index cf3ea5b16ee..92bf2a826ff 100644 --- a/lib/gitlab/internal_events.rb +++ b/lib/gitlab/internal_events.rb @@ -3,7 +3,8 @@ module Gitlab module InternalEvents UnknownEventError = Class.new(StandardError) - MissingPropertyError = Class.new(StandardError) + InvalidPropertyError = Class.new(StandardError) + InvalidMethodError = Class.new(StandardError) class << self include Gitlab::Tracking::Helpers @@ -11,23 +12,28 @@ module Gitlab def track_event(event_name, **kwargs) raise UnknownEventError, "Unknown event: #{event_name}" unless EventDefinitions.known_event?(event_name) - unique_key = EventDefinitions.unique_property(event_name) + unique_property = EventDefinitions.unique_property(event_name) + unique_method = :id - unless kwargs.has_key?(unique_key) - raise MissingPropertyError, "#{event_name} should be triggered with a '#{unique_key}' property" + unless kwargs.has_key?(unique_property) + raise InvalidPropertyError, "#{event_name} should be triggered with a named parameter '#{unique_property}'." end - UsageDataCounters::HLLRedisCounter.track_event(event_name, values: kwargs[unique_key]) + unless kwargs[unique_property].respond_to?(unique_method) + raise InvalidMethodError, "'#{unique_property}' should have a '#{unique_method}' method." + end + + unique_value = kwargs[unique_property].public_send(unique_method) # rubocop:disable GitlabSecurity/PublicSend - user_id = kwargs[:user_id] - project_id = kwargs[:project_id] - namespace_id = kwargs[:namespace_id] + UsageDataCounters::HLLRedisCounter.track_event(event_name, values: unique_value) - namespace = Namespace.find(namespace_id) if namespace_id + user = kwargs[:user] + project = kwargs[:project] + namespace = kwargs[:namespace] standard_context = Tracking::StandardContext.new( - project_id: project_id, - user_id: user_id, + project_id: project&.id, + user_id: user&.id, namespace_id: namespace&.id, plan_name: namespace&.actual_plan_name ).to_context diff --git a/lib/gitlab/internal_events/event_definitions.rb b/lib/gitlab/internal_events/event_definitions.rb index 87c6c3089f1..e1c9faa12de 100644 --- a/lib/gitlab/internal_events/event_definitions.rb +++ b/lib/gitlab/internal_events/event_definitions.rb @@ -4,7 +4,10 @@ module Gitlab module InternalEvents module EventDefinitions InvalidMetricConfiguration = Class.new(StandardError) + class << self + VALID_UNIQUE_VALUES = %w[user.id project.id namespace.id].freeze + def clear_events @events = nil end @@ -15,7 +18,15 @@ module Gitlab end def unique_property(event_name) - events[event_name] || raise(InvalidMetricConfiguration, "Unique property not defined for #{event_name}") + unique_value = events[event_name]&.to_s + + raise(InvalidMetricConfiguration, "Unique property not defined for #{event_name}") unless unique_value + + unless VALID_UNIQUE_VALUES.include?(unique_value) + raise(InvalidMetricConfiguration, "Invalid unique value '#{unique_value}' for #{event_name}") + end + + unique_value.split('.').first.to_sym end def known_event?(event_name) diff --git a/lib/gitlab/usage/metric_definition.rb b/lib/gitlab/usage/metric_definition.rb index 11d1b19f812..bd42586731e 100644 --- a/lib/gitlab/usage/metric_definition.rb +++ b/lib/gitlab/usage/metric_definition.rb @@ -48,7 +48,7 @@ module Gitlab def validate! unless skip_validation? - self.class.schemer.validate(attributes.stringify_keys).each do |error| + self.class.schemer.validate(attributes.deep_stringify_keys).each do |error| error_message = <<~ERROR_MSG Error type: #{error['type']} Data: #{error['data']} diff --git a/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb index 429ca636b2c..54464b63fce 100644 --- a/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb @@ -208,9 +208,9 @@ module Gitlab Gitlab::InternalEvents.track_event( event_name, - user_id: author.id, - project_id: project&.id, - namespace_id: namespace&.id + user: author, + project: project, + namespace: namespace ) end diff --git a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb index 1ed2e891a1f..d26b7ce951d 100644 --- a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb @@ -70,9 +70,9 @@ module Gitlab Gitlab::InternalEvents.track_event( MR_USER_CREATE_ACTION, - user_id: user.id, - project_id: project.id, - namespace_id: project.namespace_id + user: user, + project: project, + namespace: project.namespace ) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7db58b61293..f45efd46fe5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -33864,9 +33864,15 @@ msgstr "" msgid "Pipelines|Your changes have been successfully committed. Now redirecting to the new merge request page." msgstr "" +msgid "Pipelines|created" +msgstr "" + msgid "Pipelines|error" msgstr "" +msgid "Pipelines|finished" +msgstr "" + msgid "Pipelines|invalid" msgstr "" diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index aeb01603872..73ada3669a1 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -141,7 +141,7 @@ RSpec.describe 'Pipeline', :js, feature_category: :groups_and_projects do visit project_pipeline_path(project, finished_pipeline) within '[data-testid="pipeline-details-header"]' do - expect(page).to have_selector('[data-testid="pipeline-time-ago"]') + expect(page).to have_selector('[data-testid="pipeline-finished-time-ago"]') end end end @@ -151,7 +151,7 @@ RSpec.describe 'Pipeline', :js, feature_category: :groups_and_projects do visit_pipeline within '[data-testid="pipeline-details-header"]' do - expect(page).not_to have_selector('[data-testid="pipeline-time-ago"]') + expect(page).not_to have_selector('[data-testid="pipeline-finished-time-ago"]') end end end diff --git a/spec/frontend/deploy_keys/components/app_spec.js b/spec/frontend/deploy_keys/components/app_spec.js index 3dfb828b449..de4112134ce 100644 --- a/spec/frontend/deploy_keys/components/app_spec.js +++ b/spec/frontend/deploy_keys/components/app_spec.js @@ -6,6 +6,7 @@ import waitForPromises from 'helpers/wait_for_promises'; import { TEST_HOST } from 'spec/test_constants'; import deployKeysApp from '~/deploy_keys/components/app.vue'; import ConfirmModal from '~/deploy_keys/components/confirm_modal.vue'; +import NavigationTabs from '~/vue_shared/components/navigation_tabs.vue'; import eventHub from '~/deploy_keys/eventhub'; import axios from '~/lib/utils/axios_utils'; import { HTTP_STATUS_OK } from '~/lib/utils/http_status'; @@ -39,6 +40,7 @@ describe('Deploy keys app component', () => { const findLoadingIcon = () => wrapper.find('.gl-spinner'); const findKeyPanels = () => wrapper.findAll('.deploy-keys .gl-tabs-nav li'); const findModal = () => wrapper.findComponent(ConfirmModal); + const findNavigationTabs = () => wrapper.findComponent(NavigationTabs); it('renders loading icon while waiting for request', async () => { mock.onGet(TEST_ENDPOINT).reply(() => new Promise()); @@ -74,55 +76,61 @@ describe('Deploy keys app component', () => { }); }); - it('re-fetches deploy keys when enabling a key', async () => { - const key = data.public_keys[0]; + it('hasKeys returns true when there are keys', async () => { await mountComponent(); - jest.spyOn(wrapper.vm.service, 'getKeys').mockImplementation(() => {}); - jest.spyOn(wrapper.vm.service, 'enableKey').mockImplementation(() => Promise.resolve()); - eventHub.$emit('enable.key', key); - - await nextTick(); - expect(wrapper.vm.service.enableKey).toHaveBeenCalledWith(key.id); - expect(wrapper.vm.service.getKeys).toHaveBeenCalled(); + expect(findNavigationTabs().exists()).toBe(true); + expect(findLoadingIcon().exists()).toBe(false); }); - it('re-fetches deploy keys when disabling a key', async () => { + describe('enabling and disabling keys', () => { const key = data.public_keys[0]; - await mountComponent(); - jest.spyOn(wrapper.vm.service, 'getKeys').mockImplementation(() => {}); - jest.spyOn(wrapper.vm.service, 'disableKey').mockImplementation(() => Promise.resolve()); + let getMethodMock; + let putMethodMock; - eventHub.$emit('disable.key', key, () => {}); + const removeKey = async (keyEvent) => { + eventHub.$emit(keyEvent, key, () => {}); - await nextTick(); - expect(findModal().props('visible')).toBe(true); - findModal().vm.$emit('remove'); + await nextTick(); + expect(findModal().props('visible')).toBe(true); + findModal().vm.$emit('remove'); + }; - await nextTick(); - expect(wrapper.vm.service.disableKey).toHaveBeenCalledWith(key.id); - expect(wrapper.vm.service.getKeys).toHaveBeenCalled(); - }); + beforeEach(() => { + getMethodMock = jest.spyOn(axios, 'get'); + putMethodMock = jest.spyOn(axios, 'put'); + }); - it('calls disableKey when removing a key', async () => { - const key = data.public_keys[0]; - await mountComponent(); - jest.spyOn(wrapper.vm.service, 'getKeys').mockImplementation(() => {}); - jest.spyOn(wrapper.vm.service, 'disableKey').mockImplementation(() => Promise.resolve()); + afterEach(() => { + getMethodMock.mockClear(); + putMethodMock.mockClear(); + }); - eventHub.$emit('remove.key', key, () => {}); + it('re-fetches deploy keys when enabling a key', async () => { + await mountComponent(); - await nextTick(); - expect(findModal().props('visible')).toBe(true); - findModal().vm.$emit('remove'); + eventHub.$emit('enable.key', key); - await nextTick(); - expect(wrapper.vm.service.disableKey).toHaveBeenCalledWith(key.id); - expect(wrapper.vm.service.getKeys).toHaveBeenCalled(); - }); + expect(putMethodMock).toHaveBeenCalledWith(`${TEST_ENDPOINT}/${key.id}/enable`); + expect(getMethodMock).toHaveBeenCalled(); + }); - it('hasKeys returns true when there are keys', async () => { - await mountComponent(); - expect(wrapper.vm.hasKeys).toEqual(3); + it('re-fetches deploy keys when disabling a key', async () => { + await mountComponent(); + + await removeKey('disable.key'); + + expect(putMethodMock).toHaveBeenCalledWith(`${TEST_ENDPOINT}/${key.id}/disable`); + expect(getMethodMock).toHaveBeenCalled(); + }); + + it('calls disableKey when removing a key', async () => { + await mountComponent(); + + await removeKey('remove.key'); + + expect(putMethodMock).toHaveBeenCalledWith(`${TEST_ENDPOINT}/${key.id}/disable`); + expect(getMethodMock).toHaveBeenCalled(); + }); }); }); diff --git a/spec/frontend/pipelines/pipeline_details_header_spec.js b/spec/frontend/pipelines/pipeline_details_header_spec.js index deaf5c6f72f..5c9c92c8f3f 100644 --- a/spec/frontend/pipelines/pipeline_details_header_spec.js +++ b/spec/frontend/pipelines/pipeline_details_header_spec.js @@ -7,7 +7,6 @@ import waitForPromises from 'helpers/wait_for_promises'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import PipelineDetailsHeader from '~/pipelines/components/pipeline_details_header.vue'; import { BUTTON_TOOLTIP_RETRY, BUTTON_TOOLTIP_CANCEL } from '~/pipelines/constants'; -import TimeAgo from '~/pipelines/components/pipelines_list/time_ago.vue'; import CiBadgeLink from '~/vue_shared/components/ci_badge_link.vue'; import cancelPipelineMutation from '~/pipelines/graphql/mutations/cancel_pipeline.mutation.graphql'; import deletePipelineMutation from '~/pipelines/graphql/mutations/delete_pipeline.mutation.graphql'; @@ -59,8 +58,10 @@ describe('Pipeline details header', () => { const findAlert = () => wrapper.findComponent(GlAlert); const findStatus = () => wrapper.findComponent(CiBadgeLink); const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); - const findTimeAgo = () => wrapper.findComponent(TimeAgo); const findAllBadges = () => wrapper.findAllComponents(GlBadge); + const findDeleteModal = () => wrapper.findComponent(GlModal); + const findCreatedTimeAgo = () => wrapper.findByTestId('pipeline-created-time-ago'); + const findFinishedTimeAgo = () => wrapper.findByTestId('pipeline-finished-time-ago'); const findPipelineName = () => wrapper.findByTestId('pipeline-name'); const findCommitTitle = () => wrapper.findByTestId('pipeline-commit-title'); const findTotalJobs = () => wrapper.findByTestId('total-jobs'); @@ -71,7 +72,6 @@ describe('Pipeline details header', () => { const findRetryButton = () => wrapper.findByTestId('retry-pipeline'); const findCancelButton = () => wrapper.findByTestId('cancel-pipeline'); const findDeleteButton = () => wrapper.findByTestId('delete-pipeline'); - const findDeleteModal = () => wrapper.findComponent(GlModal); const findPipelineUserLink = () => wrapper.findByTestId('pipeline-user-link'); const findPipelineDuration = () => wrapper.findByTestId('pipeline-duration-text'); @@ -232,12 +232,20 @@ describe('Pipeline details header', () => { expect(findComputeCredits().exists()).toBe(false); }); - it('displays time ago', async () => { + it('does not display created time ago', async () => { + createComponent(); + + await waitForPromises(); + + expect(findCreatedTimeAgo().exists()).toBe(false); + }); + + it('displays finished time ago', async () => { createComponent(); await waitForPromises(); - expect(findTimeAgo().exists()).toBe(true); + expect(findFinishedTimeAgo().exists()).toBe(true); }); it('displays pipeline duartion text', async () => { @@ -262,8 +270,8 @@ describe('Pipeline details header', () => { expect(findComputeCredits().exists()).toBe(false); }); - it('does not display time ago', () => { - expect(findTimeAgo().exists()).toBe(false); + it('does not display finished time ago', () => { + expect(findFinishedTimeAgo().exists()).toBe(false); }); it('does not display pipeline duration text', () => { @@ -273,6 +281,10 @@ describe('Pipeline details header', () => { it('displays pipeline running text', () => { expect(findPipelineRunningText()).toBe('In progress, queued for 3,600 seconds'); }); + + it('displays created time ago', () => { + expect(findCreatedTimeAgo().exists()).toBe(true); + }); }); describe('running pipeline with duration', () => { diff --git a/spec/frontend/pipelines/time_ago_spec.js b/spec/frontend/pipelines/time_ago_spec.js index 5afe91c4784..d2aa340a980 100644 --- a/spec/frontend/pipelines/time_ago_spec.js +++ b/spec/frontend/pipelines/time_ago_spec.js @@ -65,22 +65,11 @@ describe('Timeago component', () => { expect(time.exists()).toBe(true); }); - it('should display calendar icon by default', () => { + it('should display calendar icon', () => { createComponent({ duration: 0, finished_at: '2017-04-26T12:40:23.277Z' }); expect(findCalendarIcon().exists()).toBe(true); }); - - it('should hide calendar icon if correct prop is passed', () => { - createComponent( - { duration: 0, finished_at: '2017-04-26T12:40:23.277Z' }, - { - displayCalendarIcon: false, - }, - ); - - expect(findCalendarIcon().exists()).toBe(false); - }); }); describe('without finishedTime', () => { diff --git a/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb b/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb index 517ba4d7699..a97695af398 100644 --- a/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb +++ b/spec/lib/generators/gitlab/analytics/internal_events_generator_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat let(:section) { "analytics" } let(:mr) { "https://gitlab.com/some-group/some-project/-/merge_requests/123" } let(:event) { "view_analytics_dashboard" } - let(:unique_on) { "user_id" } + let(:unique) { "user_id" } let(:time_frames) { %w[7d] } let(:include_default_identifiers) { 'yes' } let(:options) do @@ -27,11 +27,11 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat stage: stage, section: section, event: event, - unique_on: unique_on + unique: unique }.stringify_keys end - let(:key_path_7d) { "count_distinct_#{unique_on}_from_#{event}_7d" } + let(:key_path_7d) { "count_distinct_#{unique}_from_#{event}_7d" } let(:metric_definition_path_7d) { Dir.glob(File.join(temp_dir, "metrics/counts_7d/#{key_path_7d}.yml")).first } let(:metric_definition_7d) do { @@ -211,8 +211,8 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat context 'without obligatory parameter' do it 'raises error', :aggregate_failures do - %w[unique_on event mr section stage group].each do |option| - expect { described_class.new([], options.without(option)).invoke_all } + %w[unique event mr section stage group].each do |option| + expect { described_class.new([], options.without(option)).invoke_all } .to raise_error(RuntimeError) end end @@ -241,7 +241,7 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat context 'for multiple time frames' do let(:time_frames) { %w[7d 28d] } - let(:key_path_28d) { "count_distinct_#{unique_on}_from_#{event}_28d" } + let(:key_path_28d) { "count_distinct_#{unique}_from_#{event}_28d" } let(:metric_definition_path_28d) { Dir.glob(File.join(temp_dir, "metrics/counts_28d/#{key_path_28d}.yml")).first } let(:metric_definition_28d) do metric_definition_7d.merge( @@ -260,7 +260,7 @@ RSpec.describe Gitlab::Analytics::InternalEventsGenerator, :silence_stdout, feat context 'with default time frames' do let(:time_frames) { nil } - let(:key_path_28d) { "count_distinct_#{unique_on}_from_#{event}_28d" } + let(:key_path_28d) { "count_distinct_#{unique}_from_#{event}_28d" } let(:metric_definition_path_28d) { Dir.glob(File.join(temp_dir, "metrics/counts_28d/#{key_path_28d}.yml")).first } let(:metric_definition_28d) do metric_definition_7d.merge( diff --git a/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb index 6bcefa455cf..31a194ae883 100644 --- a/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb @@ -153,6 +153,25 @@ RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freez expect(calls.size).to eq(1) end + it 'does not sample a job if there are zero rows to sample' do + calls = [] + define_background_migration(migration_name, with_base_class: true, scoping: ->(relation) { + relation.none + }) do |*args| + calls << args + end + + queue_migration(migration_name, table_name, :id, + job_interval: 5.minutes, + batch_size: num_rows_in_table * 2, + sub_batch_size: num_rows_in_table * 2) + + described_class.new(result_dir: result_dir, connection: connection, + from_id: from_id).run_jobs(for_duration: 3.minutes) + + expect(calls.count).to eq(0) + end + context 'with multiple jobs to run' do let(:last_id) do Gitlab::Database::SharedModel.using_connection(connection) do diff --git a/spec/lib/gitlab/internal_events/event_definitions_spec.rb b/spec/lib/gitlab/internal_events/event_definitions_spec.rb index 8b3b7b43154..f6f79d9d906 100644 --- a/spec/lib/gitlab/internal_events/event_definitions_spec.rb +++ b/spec/lib/gitlab/internal_events/event_definitions_spec.rb @@ -29,20 +29,38 @@ RSpec.describe Gitlab::InternalEvents::EventDefinitions, feature_category: :prod end describe ".unique_property" do - context 'when event does have unique property', :aggregate_failures do - let(:events1) { { 'event1' => :user_id } } - let(:events2) { { 'event2' => :project_id } } + context 'when event has valid unique value with a period', :aggregate_failures do + let(:events1) { { 'event1' => :'user.id' } } + let(:events2) { { 'event2' => :'project.id' } } it 'is returned' do - expect(described_class.unique_property('event1')).to eq(:user_id) - expect(described_class.unique_property('event2')).to eq(:project_id) + expect(described_class.unique_property('event1')).to eq(:user) + expect(described_class.unique_property('event2')).to eq(:project) + end + end + + context 'when event has no periods in unique property', :aggregate_failures do + let(:events1) { { 'event1' => :plan_id } } + + it 'fails' do + expect { described_class.unique_property('event1') } + .to raise_error(described_class::InvalidMetricConfiguration, /Invalid unique value/) + end + end + + context 'when event has more than one period in unique property' do + let(:events1) { { 'event1' => :'project.namespace.id' } } + + it 'fails' do + expect { described_class.unique_property('event1') } + .to raise_error(described_class::InvalidMetricConfiguration, /Invalid unique value/) end end context 'when event does not have unique property' do - it 'unique_property fails' do + it 'unique fails' do expect { described_class.unique_property('event1') } - .to raise_error(described_class::InvalidMetricConfiguration) + .to raise_error(described_class::InvalidMetricConfiguration, /Unique property not defined for/) end end end diff --git a/spec/lib/gitlab/internal_events_spec.rb b/spec/lib/gitlab/internal_events_spec.rb index bd325fb0163..86215434ba4 100644 --- a/spec/lib/gitlab/internal_events_spec.rb +++ b/spec/lib/gitlab/internal_events_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Gitlab::InternalEvents, :snowplow, feature_category: :product_ana allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) allow(Gitlab::Tracking).to receive(:tracker).and_return(fake_snowplow) - allow(Gitlab::InternalEvents::EventDefinitions).to receive(:unique_property).and_return(:user_id) + allow(Gitlab::InternalEvents::EventDefinitions).to receive(:unique_property).and_return(:user) allow(fake_snowplow).to receive(:event) end @@ -36,7 +36,7 @@ RSpec.describe Gitlab::InternalEvents, :snowplow, feature_category: :product_ana end let_it_be(:user) { build(:user, id: 1) } - let_it_be(:project) { build(:project) } + let_it_be(:project) { build(:project, id: 2) } let_it_be(:namespace) { project.namespace } let(:fake_snowplow) { instance_double(Gitlab::Tracking::Destinations::Snowplow) } @@ -44,7 +44,7 @@ RSpec.describe Gitlab::InternalEvents, :snowplow, feature_category: :product_ana let(:unique_value) { user.id } it 'updates both RedisHLL and Snowplow', :aggregate_failures do - params = { user_id: user.id, project_id: project.id, namespace_id: namespace.id } + params = { user: user, project: project, namespace: namespace } described_class.track_event(event_name, **params) expect_redis_hll_tracking(event_name) @@ -52,7 +52,7 @@ RSpec.describe Gitlab::InternalEvents, :snowplow, feature_category: :product_ana end it 'rescues error', :aggregate_failures do - params = { user_id: user.id, project_id: project.id, namespace_id: namespace.id } + params = { user: user, project: project, namespace: namespace } error = StandardError.new("something went wrong") allow(fake_snowplow).to receive(:event).and_raise(error) @@ -66,18 +66,18 @@ RSpec.describe Gitlab::InternalEvents, :snowplow, feature_category: :product_ana expect { described_class.track_event(event_name, **params) }.not_to raise_error end - it 'fails on unknown event', :aggregate_failures do + it 'logs error on unknown event', :aggregate_failures do expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) .with(described_class::UnknownEventError, event_name: 'unknown_event', kwargs: {}) expect { described_class.track_event('unknown_event') }.not_to raise_error end - it 'raises on missing property' do + it 'logs error on missing property' do expect { described_class.track_event(event_name, merge_request_id: 1) }.not_to raise_error expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_for_dev_exception) - .with(described_class::MissingPropertyError, event_name: event_name, kwargs: { merge_request_id: 1 }) + .with(described_class::InvalidPropertyError, event_name: event_name, kwargs: { merge_request_id: 1 }) end context 'when unique property is missing' do @@ -95,21 +95,38 @@ RSpec.describe Gitlab::InternalEvents, :snowplow, feature_category: :product_ana context 'when unique key is defined' do let(:event_name) { 'p_ci_templates_terraform_base_latest' } - let(:user_id) { 1 } - let(:project_id) { 2 } - let(:unique_value) { project_id } + let(:unique_value) { project.id } + let(:property_name) { :project } before do allow(Gitlab::InternalEvents::EventDefinitions).to receive(:unique_property) - .with('p_ci_templates_terraform_base_latest') - .and_return(:project_id) + .with(event_name) + .and_return(property_name) end - it 'is used when logging to RedisHLL' do - described_class.track_event(event_name, user_id: user_id, project_id: project_id) + it 'is used when logging to RedisHLL', :aggregate_failures do + described_class.track_event(event_name, user: user, project: project) expect_redis_hll_tracking(event_name) expect_snowplow_tracking(event_name) end + + context 'when property is missing' do + it 'logs error' do + expect { described_class.track_event(event_name, merge_request_id: 1) }.not_to raise_error + + expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_for_dev_exception) + .with(described_class::InvalidPropertyError, event_name: event_name, kwargs: { merge_request_id: 1 }) + end + end + + context 'when method does not exist on property' do + it 'logs error on missing method' do + expect { described_class.track_event(event_name, project: "a_string") }.not_to raise_error + + expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_for_dev_exception) + .with(described_class::InvalidMethodError, event_name: event_name, kwargs: { project: 'a_string' }) + end + end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 35bf0cd2cb0..7aae8f766ea 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1673,32 +1673,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end - describe '#use_traversal_ids_for_self_and_hierarchy?' do - let_it_be(:namespace, reload: true) { create(:namespace) } - - subject { namespace.use_traversal_ids_for_self_and_hierarchy? } - - it { is_expected.to eq true } - - it_behaves_like 'disabled feature flag when traversal_ids is blank' - - context 'when use_traversal_ids_for_self_and_hierarchy feature flag is false' do - before do - stub_feature_flags(use_traversal_ids_for_self_and_hierarchy: false) - end - - it { is_expected.to eq false } - end - - context 'when use_traversal_ids? feature flag is false' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it { is_expected.to eq false } - end - end - describe '#users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } diff --git a/spec/services/users/ban_service_spec.rb b/spec/services/users/ban_service_spec.rb index 5be5de82e91..7e342340f88 100644 --- a/spec/services/users/ban_service_spec.rb +++ b/spec/services/users/ban_service_spec.rb @@ -38,6 +38,13 @@ RSpec.describe Users::BanService, feature_category: :user_management do ban_user end + + it 'tracks the event', :experiment do + expect(experiment(:phone_verification_for_low_risk_users)) + .to track(:banned).on_next_instance.with_context(user: user) + + ban_user + end end context 'when failed' do diff --git a/spec/support/helpers/database/migration_testing_helpers.rb b/spec/support/helpers/database/migration_testing_helpers.rb index 916446e66b7..87557860ab6 100644 --- a/spec/support/helpers/database/migration_testing_helpers.rb +++ b/spec/support/helpers/database/migration_testing_helpers.rb @@ -2,11 +2,13 @@ module Database module MigrationTestingHelpers - def define_background_migration(name) - klass = Class.new do + def define_background_migration(name, with_base_class: false, scoping: nil) + klass = Class.new(with_base_class ? Gitlab::BackgroundMigration::BatchedMigrationJob : Object) do # Can't simply def perform here as we won't have access to the block, # similarly can't define_method(:perform, &block) here as it would change the block receiver define_method(:perform) { |*args| yield(*args) } + + scope_to(scoping) if scoping end stub_const("Gitlab::BackgroundMigration::#{name}", klass) klass |