diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-03-20 21:14:25 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-03-20 21:14:25 +0000 |
| commit | 40a329556f63d919a68dd80bb37b087d1f0e6336 (patch) | |
| tree | 86c2712bf48f66d0e7dbe7e115e5f01c78193ad0 | |
| parent | 96b01499986c01d0c95176860a7606b7616a3836 (diff) | |
| download | gitlab-ce-40a329556f63d919a68dd80bb37b087d1f0e6336.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
53 files changed, 594 insertions, 431 deletions
diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index c5d992cab63..4d89b5718f9 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -116,6 +116,27 @@ update-storybook-yarn-cache: script: - yarn_install_script +retrieve-frontend-fixtures: + variables: + SETUP_DB: "false" + extends: + - .default-retry + - .frontend:rules:default-frontend-jobs + stage: prepare + script: + - export FIXTURES_SHA="${CI_MERGE_REQUEST_TARGET_BRANCH_SHA:-${CI_MERGE_REQUEST_DIFF_BASE_SHA:-$CI_COMMIT_SHA}}" + - source scripts/utils.sh + - source scripts/gitlab_component_helpers.sh + - | + if check_fixtures_download; then + run_timed_command "download_and_extract_fixtures" + fi + artifacts: + paths: + - tmp/tests/frontend/ + +# Download fixtures only when a merge request contains changes to only JS files +# and fixtures are present in the package registry. .frontend-fixtures-base: extends: - .default-retry @@ -125,8 +146,18 @@ update-storybook-yarn-cache: stage: fixtures needs: ["setup-test-env", "retrieve-tests-metadata"] variables: + CRYSTALBALL: "false" WEBPACK_VENDOR_DLL: "true" script: + - export FIXTURES_SHA="${CI_MERGE_REQUEST_TARGET_BRANCH_SHA:-${CI_MERGE_REQUEST_DIFF_BASE_SHA:-$CI_COMMIT_SHA}}" + - source scripts/utils.sh + - source scripts/gitlab_component_helpers.sh + - | + if check_fixtures_download; then + exit 0 + else + echo "No frontend fixtures directory, generating frontend fixtures." + fi - run_timed_command "gem install knapsack --no-document" - run_timed_command "scripts/gitaly-test-spawn" - source ./scripts/rspec_helpers.sh @@ -157,6 +188,11 @@ rspec-all frontend_fixture as-if-foss: - .frontend-fixtures-base - .frontend:rules:frontend_fixture-as-if-foss - .as-if-foss + variables: + CRYSTALBALL: "false" + WEBPACK_VENDOR_DLL: "true" + KNAPSACK_GENERATE_REPORT: "" + FLAKY_RSPEC_GENERATE_REPORT: "" needs: - !reference [.frontend-fixtures-base, needs] - "compile-test-assets as-if-foss" @@ -170,6 +206,8 @@ upload-frontend-fixtures: stage: fixtures needs: ["rspec-all frontend_fixture"] script: + - export FIXTURES_SHA="${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA:-$CI_COMMIT_SHA}" + - source scripts/utils.sh - source scripts/gitlab_component_helpers.sh - 'fixtures_archive_doesnt_exist || { echoinfo "INFO: Exiting early as package exists."; exit 0; }' - run_timed_command "create_fixtures_package" @@ -224,7 +262,7 @@ jest: extends: - .jest-base - .frontend:rules:jest - needs: ["rspec-all frontend_fixture"] + needs: ["rspec-all frontend_fixture", "retrieve-frontend-fixtures"] artifacts: name: coverage-frontend expire_in: 31d @@ -274,6 +312,7 @@ jest-integration: - run_timed_command "yarn jest:integration --ci" needs: - job: "rspec-all frontend_fixture" + - job: "retrieve-frontend-fixtures" - job: "graphql-schema-dump" coverage-frontend: @@ -360,6 +399,7 @@ startup-css-check: needs: - job: "compile-test-assets" - job: "rspec-all frontend_fixture" + - job: "retrieve-frontend-fixtures" startup-css-check as-if-foss: extends: @@ -386,6 +426,7 @@ compile-storybook: needs: - !reference [.compile-storybook-base, needs] - job: "rspec-all frontend_fixture" + - job: "retrieve-frontend-fixtures" artifacts: name: storybook expire_in: 31d diff --git a/.rubocop_todo/rake/require.yml b/.rubocop_todo/rake/require.yml index b24dd1e6540..0659dd11037 100644 --- a/.rubocop_todo/rake/require.yml +++ b/.rubocop_todo/rake/require.yml @@ -2,5 +2,4 @@ Rake/Require: Details: grace period Exclude: - - 'lib/tasks/tokens.rake' - 'qa/tasks/webdrivers.rake' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 08783f09b58..e5ec85c5ac9 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -48b0e3bafe63bc6c7786a6b1ca6c0a06d14a721f +5338468e55ae2bd6eba6ddc6021e160854348f2b @@ -526,7 +526,7 @@ gem 'flipper', '~> 0.25.0' gem 'flipper-active_record', '~> 0.25.0' gem 'flipper-active_support_cache_store', '~> 0.25.0' gem 'unleash', '~> 3.2.2' -gem 'gitlab-experiment', '~> 0.7.1' +gem 'gitlab-experiment', '~> 0.8.0' # Structured logging gem 'lograge', '~> 0.5' diff --git a/Gemfile.checksum b/Gemfile.checksum index a264fe1d13a..08ee1355967 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -206,7 +206,7 @@ {"name":"gitlab","version":"4.19.0","platform":"ruby","checksum":"3f645e3e195dbc24f0834fbf83e8ccfb2056d8e9712b01a640aad418a6949679"}, {"name":"gitlab-chronic","version":"0.10.5","platform":"ruby","checksum":"f80f18dc699b708870a80685243331290bc10cfeedb6b99c92219722f729c875"}, {"name":"gitlab-dangerfiles","version":"3.8.0","platform":"ruby","checksum":"7ef0c3205faa38a2ada19ee5b8e4012ea696611aa02564a4a95eaf3fb26d1a7e"}, -{"name":"gitlab-experiment","version":"0.7.1","platform":"ruby","checksum":"166dddb3aa83428bcaa93c35684ed01dc4d61f321fd2ae40b020806dc54a7824"}, +{"name":"gitlab-experiment","version":"0.8.0","platform":"ruby","checksum":"b4e2f73e0af19cdd899a745f5a846c1318d44054e068a8f4ac887f6b1017d3f9"}, {"name":"gitlab-fog-azure-rm","version":"1.7.0","platform":"ruby","checksum":"969c67943c54ad4c259a6acd040493f13922fbdf2211bb4eca00e71505263dc2"}, {"name":"gitlab-labkit","version":"0.31.1","platform":"ruby","checksum":"3e3a39370966b5d2739c2d9d9005c0ea27541d32cb7292e856e8bd74c720bffb"}, {"name":"gitlab-license","version":"2.2.1","platform":"ruby","checksum":"39fcf6be8b2887df8afe01b5dcbae8d08b7c5d937ff56b0fb40484a8c4f02d30"}, diff --git a/Gemfile.lock b/Gemfile.lock index f71f7f1fb1d..91ebcfe15a4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -590,7 +590,7 @@ GEM danger (>= 8.4.5) danger-gitlab (>= 8.0.0) rake - gitlab-experiment (0.7.1) + gitlab-experiment (0.8.0) activesupport (>= 3.0) request_store (>= 1.0) gitlab-fog-azure-rm (1.7.0) @@ -1716,7 +1716,7 @@ DEPENDENCIES gitaly (~> 15.9.0.pre.rc3) gitlab-chronic (~> 0.10.5) gitlab-dangerfiles (~> 3.8.0) - gitlab-experiment (~> 0.7.1) + gitlab-experiment (~> 0.8.0) gitlab-fog-azure-rm (~> 1.7.0) gitlab-labkit (~> 0.31.1) gitlab-license (~> 2.2.1) diff --git a/app/assets/javascripts/admin/abuse_reports/components/abuse_reports_filtered_search_bar.vue b/app/assets/javascripts/admin/abuse_reports/components/abuse_reports_filtered_search_bar.vue index b60fe3ae9b8..e1989cadd86 100644 --- a/app/assets/javascripts/admin/abuse_reports/components/abuse_reports_filtered_search_bar.vue +++ b/app/assets/javascripts/admin/abuse_reports/components/abuse_reports_filtered_search_bar.vue @@ -8,7 +8,7 @@ import { SORT_OPTIONS, isValidSortKey, } from '~/admin/abuse_reports/constants'; -import { buildFilteredSearchCategoryToken } from '~/admin/abuse_reports/utils'; +import { buildFilteredSearchCategoryToken, isValidStatus } from '~/admin/abuse_reports/utils'; export default { name: 'AbuseReportsFilteredSearchBar', @@ -32,7 +32,7 @@ export default { // Backend shows open reports by default if status param is not specified. // To match that behavior, update the current URL to include status=open // query when no status query is specified on load. - if (!query.status) { + if (!isValidStatus(query.status)) { query.status = 'open'; updateHistory({ url: setUrlParams(query), replace: true }); } diff --git a/app/assets/javascripts/admin/abuse_reports/utils.js b/app/assets/javascripts/admin/abuse_reports/utils.js index 84221901089..d30e8fb0ae5 100644 --- a/app/assets/javascripts/admin/abuse_reports/utils.js +++ b/app/assets/javascripts/admin/abuse_reports/utils.js @@ -1,6 +1,9 @@ -import { FILTERED_SEARCH_TOKEN_CATEGORY } from './constants'; +import { FILTERED_SEARCH_TOKEN_CATEGORY, FILTERED_SEARCH_TOKEN_STATUS } from './constants'; export const buildFilteredSearchCategoryToken = (categories) => { const options = categories.map((c) => ({ value: c, title: c })); return { ...FILTERED_SEARCH_TOKEN_CATEGORY, options }; }; + +export const isValidStatus = (status) => + FILTERED_SEARCH_TOKEN_STATUS.options.map((o) => o.value).includes(status); diff --git a/app/assets/javascripts/issues/show/components/app.vue b/app/assets/javascripts/issues/show/components/app.vue index 15f97222971..6622fb7aa7b 100644 --- a/app/assets/javascripts/issues/show/components/app.vue +++ b/app/assets/javascripts/issues/show/components/app.vue @@ -95,10 +95,10 @@ export default { required: false, default: '', }, - initialTaskStatus: { - type: String, + initialTaskCompletionStatus: { + type: Object, required: false, - default: '', + default: () => ({}), }, updatedAt: { type: String, @@ -197,7 +197,7 @@ export default { updatedAt: this.updatedAt, updatedByName: this.updatedByName, updatedByPath: this.updatedByPath, - taskStatus: this.initialTaskStatus, + taskCompletionStatus: this.initialTaskCompletionStatus, lock_version: this.lockVersion, }); @@ -222,9 +222,6 @@ export default { formState() { return this.store.formState; }, - hasUpdated() { - return Boolean(this.state.updatedAt); - }, issueChanged() { const { store: { @@ -557,7 +554,6 @@ export default { :description-html="state.descriptionHtml" :description-text="state.descriptionText" :updated-at="state.updatedAt" - :task-status="state.taskStatus" :issuable-type="issuableType" :update-url="updateEndpoint" :lock-version="state.lock_version" @@ -570,7 +566,7 @@ export default { /> <edited-component - v-if="hasUpdated" + :task-completion-status="state.taskCompletionStatus" :updated-at="state.updatedAt" :updated-by-name="state.updatedByName" :updated-by-path="state.updatedByPath" diff --git a/app/assets/javascripts/issues/show/components/description.vue b/app/assets/javascripts/issues/show/components/description.vue index bdee6c5fe9a..6412a13e3e6 100644 --- a/app/assets/javascripts/issues/show/components/description.vue +++ b/app/assets/javascripts/issues/show/components/description.vue @@ -1,6 +1,5 @@ <script> import { GlToast } from '@gitlab/ui'; -import $ from 'jquery'; import Sortable from 'sortablejs'; import Vue from 'vue'; import getIssueDetailsQuery from 'ee_else_ce/work_items/graphql/get_issue_details.query.graphql'; @@ -59,11 +58,6 @@ export default { required: false, default: '', }, - taskStatus: { - type: String, - required: false, - default: '', - }, issuableType: { type: String, required: false, @@ -148,16 +142,12 @@ export default { this.renderGFM(); }); }, - taskStatus() { - this.updateTaskStatusText(); - }, }, mounted() { eventHub.$on('convert-task-list-item', this.convertTaskListItem); eventHub.$on('delete-task-list-item', this.deleteTaskListItem); this.renderGFM(); - this.updateTaskStatusText(); }, beforeDestroy() { eventHub.$off('convert-task-list-item', this.convertTaskListItem); @@ -282,24 +272,6 @@ export default { this.$emit('taskListUpdateFailed'); }, - updateTaskStatusText() { - const taskRegexMatches = this.taskStatus.match(/(\d+) of ((?!0)\d+)/); - const $issuableHeader = $('.issuable-meta'); - const $tasks = $('#task_status', $issuableHeader); - const $tasksShort = $('#task_status_short', $issuableHeader); - - if (taskRegexMatches) { - $tasks.text(this.taskStatus); - $tasksShort.text( - `${taskRegexMatches[1]}/${taskRegexMatches[2]} checklist item${ - taskRegexMatches[2] > 1 ? 's' : '' - }`, - ); - } else { - $tasks.text(''); - $tasksShort.text(''); - } - }, createTaskListItemActions(provide) { const app = new Vue({ el: document.createElement('div'), diff --git a/app/assets/javascripts/issues/show/components/edited.vue b/app/assets/javascripts/issues/show/components/edited.vue index 5138a4530e9..6a0edb59b65 100644 --- a/app/assets/javascripts/issues/show/components/edited.vue +++ b/app/assets/javascripts/issues/show/components/edited.vue @@ -1,13 +1,20 @@ <script> -import { GlSprintf } from '@gitlab/ui'; +import { GlLink, GlSprintf } from '@gitlab/ui'; +import { n__, sprintf } from '~/locale'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; export default { components: { TimeAgoTooltip, + GlLink, GlSprintf, }, props: { + taskCompletionStatus: { + type: Object, + required: false, + default: () => ({}), + }, updatedAt: { type: String, required: false, @@ -25,36 +32,61 @@ export default { }, }, computed: { + completedCount() { + return this.taskCompletionStatus.completed_count; + }, + count() { + return this.taskCompletionStatus.count; + }, hasUpdatedBy() { return this.updatedByName && this.updatedByPath; }, + showCheck() { + return this.completedCount === this.count; + }, + taskStatus() { + const { completedCount, count } = this; + if (!count) { + return undefined; + } + + return sprintf( + n__( + '%{completedCount} of %{count} checklist item completed', + '%{completedCount} of %{count} checklist items completed', + count, + ), + { completedCount, count }, + ); + }, }, }; </script> <template> <small class="edited-text js-issue-widgets"> - <gl-sprintf v-if="!hasUpdatedBy" :message="__('Edited %{timeago}')"> - <template #timeago> - <time-ago-tooltip :time="updatedAt" tooltip-placement="bottom" /> - </template> - </gl-sprintf> - <gl-sprintf v-else-if="!updatedAt" :message="__('Edited by %{author}')"> - <template #author> - <a :href="updatedByPath" class="author-link gl-hover-text-decoration-underline"> - <span>{{ updatedByName }}</span> - </a> - </template> - </gl-sprintf> - <gl-sprintf v-else :message="__('Edited %{timeago} by %{author}')"> - <template #timeago> - <time-ago-tooltip :time="updatedAt" tooltip-placement="bottom" /> - </template> - <template #author> - <a :href="updatedByPath" class="author-link gl-hover-text-decoration-underline"> - <span>{{ updatedByName }}</span> - </a> - </template> - </gl-sprintf> + <template v-if="taskStatus"> + <template v-if="showCheck">✓</template> + {{ taskStatus }} + <template v-if="updatedAt">·</template> + </template> + + <template v-if="updatedAt"> + <gl-sprintf v-if="!hasUpdatedBy" :message="__('Edited %{timeago}')"> + <template #timeago> + <time-ago-tooltip :time="updatedAt" tooltip-placement="bottom" /> + </template> + </gl-sprintf> + <gl-sprintf v-else :message="__('Edited %{timeago} by %{author}')"> + <template #timeago> + <time-ago-tooltip :time="updatedAt" tooltip-placement="bottom" /> + </template> + <template #author> + <gl-link :href="updatedByPath" class="gl-font-sm gl-hover-text-gray-900 gl-text-gray-700"> + {{ updatedByName }} + </gl-link> + </template> + </gl-sprintf> + </template> </small> </template> diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index d364daf93c3..b8d47586a15 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -76,7 +76,7 @@ module IssuableActions title_text: issuable.title, description: view_context.markdown_field(issuable, :description), description_text: issuable.description, - task_status: issuable.task_status, + task_completion_status: issuable.task_completion_status, lock_version: issuable.lock_version } diff --git a/app/controllers/concerns/product_analytics_tracking.rb b/app/controllers/concerns/product_analytics_tracking.rb index 5ed2b2a82eb..fc33770b4d8 100644 --- a/app/controllers/concerns/product_analytics_tracking.rb +++ b/app/controllers/concerns/product_analytics_tracking.rb @@ -5,30 +5,6 @@ module ProductAnalyticsTracking include RedisTracking extend ActiveSupport::Concern - MIGRATED_EVENTS = %w[ - g_analytics_valuestream - i_search_paid - i_search_total - i_search_advanced - i_ecosystem_jira_service_list_issues - users_viewing_analytics_group_devops_adoption - i_analytics_dev_ops_adoption - i_analytics_dev_ops_score - p_analytics_merge_request - i_analytics_instance_statistics - g_analytics_contribution - p_analytics_pipelines - p_analytics_code_reviews - p_analytics_valuestream - p_analytics_insights - p_analytics_issues - p_analytics_repo - g_analytics_insights - g_analytics_issues - g_analytics_productivity - i_analytics_cohorts - ].freeze - class_methods do def track_event(*controller_actions, name:, action: nil, label: nil, conditions: nil, destinations: [:redis_hll], &block) custom_conditions = [:trackable_html_request?, *conditions] @@ -44,7 +20,7 @@ module ProductAnalyticsTracking def route_events_to(destinations, name, action, label, &block) track_unique_redis_hll_event(name, &block) if destinations.include?(:redis_hll) - return unless destinations.include?(:snowplow) && event_enabled?(name) + return unless destinations.include?(:snowplow) raise "action is required when destination is snowplow" unless action raise "label is required when destination is snowplow" unless label @@ -63,18 +39,4 @@ module ProductAnalyticsTracking **optional_arguments ) end - - def event_enabled?(event) - return true if MIGRATED_EVENTS.include?(event) - - events_to_ff = { - g_edit_by_sfe: :_phase4, - g_compliance_dashboard: :_phase4, - g_compliance_audit_events: :_phase4, - i_compliance_audit_events: :_phase4, - i_compliance_credential_inventory: :_phase4 - } - - Feature.enabled?("route_hll_to_snowplow#{events_to_ff[event.to_sym]}", tracking_namespace_source) - end end diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 6fdd4906613..5b1a7f0bab7 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -339,6 +339,7 @@ class Projects::PipelinesController < Projects::ApplicationController return if helpers.has_gitlab_ci?(project) experiment(:runners_availability_section, namespace: project.root_ancestor) do |e| + e.control {} e.candidate {} end end diff --git a/app/finders/abuse_reports_finder.rb b/app/finders/abuse_reports_finder.rb index c3159198261..6a6d0413194 100644 --- a/app/finders/abuse_reports_finder.rb +++ b/app/finders/abuse_reports_finder.rb @@ -3,6 +3,7 @@ class AbuseReportsFinder attr_reader :params, :reports + DEFAULT_STATUS_FILTER = 'open' DEFAULT_SORT = 'created_at_desc' ALLOWED_SORT = [DEFAULT_SORT, *%w[created_at_asc updated_at_desc updated_at_asc]].freeze @@ -30,9 +31,13 @@ class AbuseReportsFinder end def filter_by_status + return unless Feature.enabled?(:abuse_reports_list) return unless params[:status].present? - case params[:status] + status = params[:status] + status = DEFAULT_STATUS_FILTER unless status.in?(AbuseReport.statuses.keys) + + case status when 'open' @reports = @reports.open when 'closed' diff --git a/app/helpers/ci/pipelines_helper.rb b/app/helpers/ci/pipelines_helper.rb index 823332c3d1d..90806df1d64 100644 --- a/app/helpers/ci/pipelines_helper.rb +++ b/app/helpers/ci/pipelines_helper.rb @@ -105,10 +105,12 @@ module Ci } experiment(:runners_availability_section, namespace: project.root_ancestor) do |e| + e.control {} # rubocop:disable Lint/EmptyBlock e.candidate { data[:any_runners_available] = project.active_runners.exists?.to_s } end experiment(:ios_specific_templates, actor: current_user, project: project, sticky_to: project) do |e| + e.control {} # rubocop:disable Lint/EmptyBlock e.candidate do data[:registration_token] = project.runners_token if can?(current_user, :register_project_runners, project) data[:ios_runners_available] = (project.shared_runners_available? && Gitlab.com?).to_s diff --git a/app/helpers/ide_helper.rb b/app/helpers/ide_helper.rb index 063eef41f77..2f25afb8057 100644 --- a/app/helpers/ide_helper.rb +++ b/app/helpers/ide_helper.rb @@ -72,6 +72,7 @@ module IdeHelper def enable_environments_guidance?(project) experiment(:in_product_guidance_environments_webide, project: project) do |e| + e.control { 'false' } e.candidate { !has_dismissed_ide_environments_callout? } e.run diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 9c68f54f42e..a2e8d268455 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -173,9 +173,6 @@ module IssuablesHelper output << content_tag(:span, (sprite_icon('first-contribution', css_class: 'gl-icon gl-vertical-align-middle') if issuable.first_contribution?), class: 'has-tooltip gl-ml-2', title: _('1st contribution!')) - output << content_tag(:span, (issuable.task_status if issuable.tasks?), id: "task_status", class: "d-none d-md-inline-block gl-ml-3") - output << content_tag(:span, (issuable.task_status_short if issuable.tasks?), id: "task_status_short", class: "d-md-none") - output.join.html_safe end @@ -252,7 +249,7 @@ module IssuablesHelper initialTitleText: issuable.title, initialDescriptionHtml: markdown_field(issuable, :description), initialDescriptionText: issuable.description, - initialTaskStatus: issuable.task_status + initialTaskCompletionStatus: issuable.task_completion_status } data.merge!(issue_only_initial_data(issuable)) data.merge!(path_data(parent)) diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 8898f7feb17..39d0d0a7923 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -164,19 +164,17 @@ module Notes track_note_creation_in_ipynb(note) track_note_creation_visual_review(note) - if Feature.enabled?(:route_hll_to_snowplow_phase4, project&.namespace) && note.for_commit? - metric_key_path = 'counts.commit_comment' - - Gitlab::Tracking.event( - 'Notes::CreateService', - 'create_commit_comment', - project: project, - namespace: project&.namespace, - user: user, - label: metric_key_path, - context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis, key_path: metric_key_path).to_context] - ) - end + metric_key_path = 'counts.commit_comment' + + Gitlab::Tracking.event( + 'Notes::CreateService', + 'create_commit_comment', + project: project, + namespace: project&.namespace, + user: user, + label: metric_key_path, + context: [Gitlab::Tracking::ServicePingContext.new(data_source: :redis, key_path: metric_key_path).to_context] + ) end def tracking_data_for(note) diff --git a/app/workers/namespaces/root_statistics_worker.rb b/app/workers/namespaces/root_statistics_worker.rb index 02b3468c052..c8527182d35 100644 --- a/app/workers/namespaces/root_statistics_worker.rb +++ b/app/workers/namespaces/root_statistics_worker.rb @@ -16,15 +16,23 @@ module Namespaces def perform(namespace_id) namespace = Namespace.find(namespace_id) + if Feature.enabled?(:remove_aggregation_schedule_lease, namespace) + Namespaces::StatisticsRefresherService.new.execute(namespace) + else + refresh_through_namespace_aggregation_schedule(namespace) + end + + notify_storage_usage(namespace) + rescue ::Namespaces::StatisticsRefresherService::RefresherError, ActiveRecord::RecordNotFound => ex + Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id, namespace: namespace&.full_path) + end + + def refresh_through_namespace_aggregation_schedule(namespace) return unless namespace.aggregation_scheduled? Namespaces::StatisticsRefresherService.new.execute(namespace) namespace.aggregation_schedule.destroy - - notify_storage_usage(namespace) - rescue ::Namespaces::StatisticsRefresherService::RefresherError, ActiveRecord::RecordNotFound => ex - Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id, namespace: namespace&.full_path) end private diff --git a/app/workers/namespaces/schedule_aggregation_worker.rb b/app/workers/namespaces/schedule_aggregation_worker.rb index 7cd7f5223d6..bf48eb8180e 100644 --- a/app/workers/namespaces/schedule_aggregation_worker.rb +++ b/app/workers/namespaces/schedule_aggregation_worker.rb @@ -13,16 +13,24 @@ module Namespaces idempotent! def perform(namespace_id) - return unless aggregation_schedules_table_exists? - namespace = Namespace.find(namespace_id) root_ancestor = namespace.root_ancestor + if Feature.enabled?(:remove_aggregation_schedule_lease, root_ancestor) + Namespaces::RootStatisticsWorker.perform_async(root_ancestor.id) + else + schedule_through_aggregation_schedules_table(root_ancestor) + end + rescue ActiveRecord::RecordNotFound => ex + Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id) + end + + def schedule_through_aggregation_schedules_table(root_ancestor) + return unless aggregation_schedules_table_exists? + return if root_ancestor.aggregation_scheduled? Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: root_ancestor.id) - rescue ActiveRecord::RecordNotFound => ex - Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id) end private diff --git a/config/feature_flags/development/remove_aggregation_schedule_lease.yml b/config/feature_flags/development/remove_aggregation_schedule_lease.yml new file mode 100644 index 00000000000..77367be3200 --- /dev/null +++ b/config/feature_flags/development/remove_aggregation_schedule_lease.yml @@ -0,0 +1,8 @@ +--- +name: remove_aggregation_schedule_lease +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113959 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/395018 +milestone: '15.10' +type: development +group: group::utilization +default_enabled: false diff --git a/config/feature_flags/development/route_hll_to_snowplow_phase4.yml b/config/feature_flags/development/route_hll_to_snowplow_phase4.yml deleted file mode 100644 index a4109b15533..00000000000 --- a/config/feature_flags/development/route_hll_to_snowplow_phase4.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: route_hll_to_snowplow_phase4 -introduced_by_url: "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/103528" -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/366767 -milestone: '15.7' -type: development -group: group::product intelligence -default_enabled: false diff --git a/doc/administration/geo/disaster_recovery/background_verification.md b/doc/administration/geo/disaster_recovery/background_verification.md index 99ac95cd536..ea4523c66e6 100644 --- a/doc/administration/geo/disaster_recovery/background_verification.md +++ b/doc/administration/geo/disaster_recovery/background_verification.md @@ -6,12 +6,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Automatic background verification **(PREMIUM SELF)** -NOTE: -Automatic background verification of repositories and wikis was added in -GitLab EE 10.6 but is enabled by default only on GitLab EE 11.1. You can -disable or enable this feature manually by following -[these instructions](#disabling-or-enabling-the-automatic-background-verification). - Automatic background verification ensures that the transferred data matches a calculated checksum. If the checksum of the data on the **primary** site matches checksum of the data on the **secondary** site, the data transferred successfully. Following a planned failover, @@ -179,19 +173,5 @@ If the **primary** and **secondary** sites have a checksum verification mismatch ## Current limitations -Automatic background verification doesn't cover attachments, LFS objects, -job artifacts, and user uploads in file storage. You can keep track of the -progress to include them in [Geo: Verify all replicated data](https://gitlab.com/groups/gitlab-org/-/epics/1430). - -For now, you can verify their integrity -manually by following [these instructions](../../raketasks/check.md) on both -sites, and comparing the output between them. - -In GitLab EE 12.1, Geo calculates checksums for attachments, LFS objects, and -archived traces on secondary sites after the transfer, compares it with the -stored checksums, and rejects transfers if mismatched. Geo -currently does not support an automatic way to verify these data if they have -been synced before GitLab EE 12.1. - -Data in object storage is **not verified**, as the object store is responsible -for ensuring the integrity of the data. +For more information on what replication and verification methods are supported, +see the [supported Geo data types](../replication/datatypes.md). diff --git a/doc/architecture/blueprints/ci_pipeline_components/index.md b/doc/architecture/blueprints/ci_pipeline_components/index.md index c1094bc4290..6d8e87d2dd4 100644 --- a/doc/architecture/blueprints/ci_pipeline_components/index.md +++ b/doc/architecture/blueprints/ci_pipeline_components/index.md @@ -703,6 +703,6 @@ Domain experts: | Area | Who |------------------------------|------------------------| | Verify / Pipeline authoring | Avielle Wolfe | -| Verify / Pipeline authoring | Laura Montemayor-Rodriguez | +| Verify / Pipeline authoring | Laura Montemayor | <!-- vale gitlab.Spelling = YES --> diff --git a/doc/install/docker.md b/doc/install/docker.md index 40eb3a9796e..8d3c81d72c8 100644 --- a/doc/install/docker.md +++ b/doc/install/docker.md @@ -456,6 +456,30 @@ web browser under `<hostIP>:8929` and push using SSH under the port `2289`. A `docker-compose.yml` example that uses different ports can be found in the [Docker compose](#install-gitlab-using-docker-compose) section. +### Configure multiple database connections + +In GitLab 16.0, GitLab will default to using two database connections that point to the same PostgreSQL database. + +If you want to opt-in for this feature: + +1. Edit `/etc/gitlab/gitlab.rb` inside the container: + + ```shell + sudo docker exec -it gitlab editor /etc/gitlab/gitlab.rb + ``` + +1. Add the following line: + + ```ruby + gitlab_rails['databases']['ci']['enable'] = true + ``` + +1. Restart the container: + +```shell +sudo docker restart gitlab +``` + ## Upgrade In most cases, upgrading GitLab is as easy as downloading the newest Docker diff --git a/doc/user/compliance/license_scanning_of_cyclonedx_files/index.md b/doc/user/compliance/license_scanning_of_cyclonedx_files/index.md index 0ad73f9939d..6964234aecb 100644 --- a/doc/user/compliance/license_scanning_of_cyclonedx_files/index.md +++ b/doc/user/compliance/license_scanning_of_cyclonedx_files/index.md @@ -7,10 +7,9 @@ info: To determine the technical writer assigned to the Stage/Group associated w # License scanning of CycloneDX files **(ULTIMATE)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/384932) in GitLab 15.9 [with two flags](../../../administration/feature_flags.md) named `license_scanning_sbom_scanner` and `package_metadata_synchronization`. Both flags are disabled by default and both flags must be enabled for this feature to work. - -FLAG: -On self-managed GitLab, this feature is not available. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/384932) in GitLab 15.9 for GitLab SaaS [with two flags](../../../administration/feature_flags.md) named `license_scanning_sbom_scanner` and `package_metadata_synchronization`. Both flags are disabled by default and both flags must be enabled for this feature to work. +> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/385173) in GitLab 15.10. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/385173) in GitLab 15.10 for self-managed GitLab [with two flags](../../../administration/feature_flags.md) named `license_scanning_sbom_scanner` and `package_metadata_synchronization`, both of which must be enabled for this feature to work. The flags are disabled by default due to the initial performance load when the license database is first imported. Work to improve performance is being tracked in [issue 397670](https://gitlab.com/gitlab-org/gitlab/-/issues/397670). To detect the licenses in use, License Compliance relies on running the [Dependency Scanning CI Jobs](../../application_security/dependency_scanning/index.md), diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index 04dfdbc6892..61f248a0c63 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -52,7 +52,7 @@ If you have any questions on configuring the SAML app, contact your provider's s ### Set up Azure -To set up SSO with Azure as your identification provider: +To set up SSO with Azure as your identity provider: 1. In GitLab, on the top bar, select **Main menu > Groups** and find your group. 1. On the left sidebar, select **Settings > SAML SSO**. @@ -78,19 +78,23 @@ To set up SSO with Azure as your identification provider: <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> View a demo of [SCIM provisioning on Azure using SAML SSO for groups](https://youtu.be/24-ZxmTeEBU). The `objectID` mapping is outdated in this video. Follow the [SCIM documentation](scim_setup.md#configure-azure-active-directory) instead. -View an [example configuration page](example_saml_config.md#azure-active-directory). +For more information, see an [example configuration page](example_saml_config.md#azure-active-directory). ### Set up Google Workspace -1. [Set up SSO with Google as your identity provider](https://support.google.com/a/answer/6087519?hl=en). - The following GitLab settings correspond to the Google Workspace fields. +To set up Google Workspace as your identity provider: - | GitLab setting | Google Workspace field | - |:-------------------------------------|:-----------------------| - | Identifier | **Entity ID** | - | Assertion consumer service URL | **ACS URL** | - | GitLab single sign-on URL | **Start URL** | - | Identity provider single sign-on URL | **SSO URL** | +1. In GitLab, on the top bar, select **Main menu > Groups** and find your group. +1. On the left sidebar, select **Settings > SAML SSO**. +1. Note the information on this page. +1. Follow the instructions for [setting up SSO with Google as your identity provider](https://support.google.com/a/answer/6087519?hl=en). The following GitLab settings correspond to the Google Workspace fields. + + | GitLab setting | Google Workspace field | + |:-----------------------------------------|:-----------------------| + | **Identifier** | **Entity ID** | + | **Assertion consumer service URL** | **ACS URL** | + | **GitLab single sign-on URL** | **Start URL** | + | **Identity provider single sign-on URL** | **SSO URL** | 1. Google Workspace displays a SHA256 fingerprint. To retrieve the SHA1 fingerprint required by GitLab to [configure SAML](#configure-gitlab): @@ -111,7 +115,7 @@ View an [example configuration page](example_saml_config.md#azure-active-directo On the GitLab SAML SSO page, when you select **Verify SAML Configuration**, disregard the warning that recommends setting the **NameID** format to `persistent`. -For details, see the [example configuration page](example_saml_config.md#google-workspace). +For more information, see an [example configuration page](example_saml_config.md#google-workspace). ### Set up Okta diff --git a/doc/user/project/wiki/group.md b/doc/user/project/wiki/group.md index 1513ea18ed2..de6aed52fd8 100644 --- a/doc/user/project/wiki/group.md +++ b/doc/user/project/wiki/group.md @@ -71,7 +71,7 @@ To open group settings: 1. On the left sidebar, select **Settings > General**. 1. Expand **Permissions and group features**. 1. Scroll to **Wiki** and select one of these options: - - **Enabled**: Everyone who can access the group can access the wiki. + - **Enabled**: For public groups, everyone can access the wiki. For internal groups, only authenticated users can access the wiki. - **Private**: Only group members can access the wiki. - **Disabled**: The wiki isn't accessible, and cannot be downloaded. 1. Select **Save changes**. diff --git a/lib/tasks/tokens.rake b/lib/tasks/tokens.rake index ff14ab51b49..81e24f4f7b6 100644 --- a/lib/tasks/tokens.rake +++ b/lib/tasks/tokens.rake @@ -1,11 +1,11 @@ # frozen_string_literal: true -require_relative '../../app/models/concerns/token_authenticatable' -require_relative '../../app/models/concerns/token_authenticatable_strategies/base' -require_relative '../../app/models/concerns/token_authenticatable_strategies/insecure' -require_relative '../../app/models/concerns/token_authenticatable_strategies/digest' - namespace :tokens do + require_relative '../../app/models/concerns/token_authenticatable' + require_relative '../../app/models/concerns/token_authenticatable_strategies/base' + require_relative '../../app/models/concerns/token_authenticatable_strategies/insecure' + require_relative '../../app/models/concerns/token_authenticatable_strategies/digest' + desc "Reset all GitLab incoming email tokens" task reset_all_email: :environment do reset_all_users_token(:reset_incoming_email_token!) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4355965db51..e62f0de5bf0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15652,9 +15652,6 @@ msgstr "" msgid "Edited %{timeago} by %{author}" msgstr "" -msgid "Edited by %{author}" -msgstr "" - msgid "Editing" msgstr "" @@ -35312,6 +35309,9 @@ msgstr "" msgid "ProtectedEnvironments|Unified approval rules have been removed from the settings UI" msgstr "" +msgid "ProtectedEnvironments|Unprotect" +msgstr "" + msgid "ProtectedEnvironments|Users" msgstr "" diff --git a/scripts/gitlab_component_helpers.sh b/scripts/gitlab_component_helpers.sh index 309e339de01..798135a5490 100644 --- a/scripts/gitlab_component_helpers.sh +++ b/scripts/gitlab_component_helpers.sh @@ -52,22 +52,9 @@ export GITLAB_ASSETS_PACKAGE="assets-${NODE_ENV}-${GITLAB_EDITION}-${GITLAB_ASSE export GITLAB_ASSETS_PACKAGE_URL="${API_PACKAGES_BASE_URL}/assets/${NODE_ENV}-${GITLAB_EDITION}-${GITLAB_ASSETS_HASH}/${GITLAB_ASSETS_PACKAGE}" # Fixtures constants - -# Export the SHA variable for updating/downloading fixture packages, using the following order of precedence: -# 1. If MERGE_BASE_SHA is defined, use its value. -# 2. If CI_MERGE_REQUEST_SOURCE_BRANCH_SHA is defined, use its value for merge request pipelines. -# 3. Otherwise, use the value of CI_COMMIT_SHA for default branch pipelines or merge requests with detached pipelines. -if [ -n "${MERGE_BASE_SHA:-}" ]; then - export FIXTURES_SHA="${MERGE_BASE_SHA}" -elif [ -n "${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA:-}" ]; then - export FIXTURES_SHA="${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}" -else - export FIXTURES_SHA="${CI_COMMIT_SHA}" -fi - export FIXTURES_PATH="tmp/tests/frontend/**/*" -export FIXTURES_PACKAGE="fixtures-${FIXTURES_SHA}.tar.gz" -export FIXTURES_PACKAGE_URL="${API_PACKAGES_BASE_URL}/fixtures/${FIXTURES_SHA}/${FIXTURES_PACKAGE}" +export FIXTURES_PACKAGE="fixtures-${FIXTURES_SHA:-}.tar.gz" +export FIXTURES_PACKAGE_URL="${API_PACKAGES_BASE_URL}/fixtures/${FIXTURES_SHA:-}/${FIXTURES_PACKAGE}" # Generic helper functions function archive_doesnt_exist() { @@ -75,7 +62,13 @@ function archive_doesnt_exist() { status=$(curl -I --silent --retry 3 --output /dev/null -w "%{http_code}" "${package_url}") - [[ "${status}" != "200" ]] + if [[ "${status}" != "200" ]]; then + echoinfo "The archive was not found. The server returned status ${status}." + return 0 + else + echoinfo "The archive was found. The server returned status ${status}." + return 1 + fi } function create_package() { @@ -167,10 +160,50 @@ function upload_gitlab_assets_package() { } # Fixtures functions +function download_and_extract_fixtures() { + read_curl_package "${FIXTURES_PACKAGE_URL}" | extract_package +} + function fixtures_archive_doesnt_exist() { + echoinfo "Checking if the package is available at ${FIXTURES_PACKAGE_URL} ..." + archive_doesnt_exist "${FIXTURES_PACKAGE_URL}" } +function fixtures_directory_exists() { + local fixtures_directory="tmp/tests/frontend/" + + if [[ -d "${fixtures_directory}" ]]; then + echo "${fixtures_directory} directory exists" + return 0 + else + echo "${fixtures_directory} directory does not exist" + return 1 + fi +} + +function check_fixtures_download() { + if [[ "${REUSE_FRONTEND_FIXTURES_ENABLED:-}" != "true" ]]; then + return 1 + fi + + # Note: Currently, reusing frontend fixtures is only supported in EE. + # Other projects will be supported through this issue in the future: https://gitlab.com/gitlab-org/gitlab/-/issues/393615. + if [[ "${CI_PROJECT_NAME}" != "gitlab" ]] || [[ "${CI_JOB_NAME}" =~ "foss" ]]; then + return 1 + fi + + if [[ -z "${CI_MERGE_REQUEST_IID:-}" ]]; then + return 1 + else + if only_js_files_changed && ! fixtures_archive_doesnt_exist; then + return 0 + else + return 1 + fi + fi +} + function create_fixtures_package() { create_package "${FIXTURES_PACKAGE}" "${FIXTURES_PATH}" } @@ -178,3 +211,27 @@ function create_fixtures_package() { function upload_fixtures_package() { upload_package "${FIXTURES_PACKAGE}" "${FIXTURES_PACKAGE_URL}" } + +function only_js_files_changed { + local target_branch_sha="${CI_MERGE_REQUEST_TARGET_BRANCH_SHA:-}" + local source_branch_sha="${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA:-}" + + if [[ -z "${target_branch_sha}" || -z "${source_branch_sha}" ]]; then + echoinfo "The commit hash(es) provided are missing or are empty." + echoinfo "Please provide valid commit hash(es)." + return 1 + fi + + local changed_files + changed_files=$(git diff --name-only "${target_branch_sha}..${source_branch_sha}") + + for file in $changed_files; do + if [[ ! $file = *.js ]]; then + echoinfo "Changes were made to files other than JS files" + return 1 + fi + done + + echoinfo "Only JS files were changed" + return 0 +} diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh index de735e03db0..bd2c01dac5e 100644 --- a/scripts/rspec_helpers.sh +++ b/scripts/rspec_helpers.sh @@ -164,7 +164,8 @@ function rspec_paralellized_job() { local test_tool="${job_name[0]}" local test_level="${job_name[1]}" local report_name=$(echo "${CI_JOB_NAME}" | sed -E 's|[/ ]|_|g') # e.g. 'rspec unit pg12 1/24' would become 'rspec_unit_pg12_1_24' - local rspec_opts="${1}" + local rspec_opts="${1:-}" + local rspec_tests_mapping_enabled="${RSPEC_TESTS_MAPPING_ENABLED:-}" local spec_folder_prefixes="" local rspec_flaky_folder_path="$(dirname "${FLAKY_RSPEC_SUITE_REPORT_PATH}")/" local knapsack_folder_path="$(dirname "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}")/" @@ -218,7 +219,7 @@ function rspec_paralellized_job() { debug_rspec_variables - if [[ -n "${RSPEC_TESTS_MAPPING_ENABLED}" ]]; then + if [[ -n "${rspec_tests_mapping_enabled}" ]]; then tooling/bin/parallel_rspec --rspec_args "$(rspec_args "${rspec_opts}")" --filter "${RSPEC_TESTS_FILTER_FILE}" || rspec_run_status=$? else tooling/bin/parallel_rspec --rspec_args "$(rspec_args "${rspec_opts}")" || rspec_run_status=$? diff --git a/spec/controllers/concerns/product_analytics_tracking_spec.rb b/spec/controllers/concerns/product_analytics_tracking_spec.rb index b0074b52aa2..65c2c77c027 100644 --- a/spec/controllers/concerns/product_analytics_tracking_spec.rb +++ b/spec/controllers/concerns/product_analytics_tracking_spec.rb @@ -107,20 +107,6 @@ RSpec.describe ProductAnalyticsTracking, :snowplow, feature_category: :product_a expect_snowplow_tracking(user) end - context 'when FF is disabled' do - before do - stub_const("#{described_class}::MIGRATED_EVENTS", []) - allow(Feature).to receive(:enabled?).and_call_original - allow(Feature).to receive(:enabled?).with('route_hll_to_snowplow', anything).and_return(false) - end - - it 'doesnt track snowplow event' do - get :index - - expect_no_snowplow_event - end - end - it 'tracks the event if DNT is not enabled' do stub_do_not_track('0') diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index c091badd09d..3f14317a84b 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -360,7 +360,6 @@ RSpec.describe Projects::BlobController do let(:namespace) { project.namespace.reload } let(:property) { target_event } let(:label) { 'usage_activity_by_stage_monthly.create.action_monthly_active_users_sfe_edit' } - let(:feature_flag_name) { 'route_hll_to_snowplow_phase4' } end end end @@ -520,7 +519,6 @@ RSpec.describe Projects::BlobController do let(:namespace) { project.namespace } let(:property) { target_event } let(:label) { 'usage_activity_by_stage_monthly.create.action_monthly_active_users_sfe_edit' } - let(:feature_flag_name) { 'route_hll_to_snowplow_phase4' } end it 'redirects to blob' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index f1fe1940414..6747678b6fb 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -730,7 +730,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do go(id: issue.iid) expect(json_response).to include('title_text', 'description', 'description_text') - expect(json_response).to include('task_status', 'lock_version') + expect(json_response).to include('task_completion_status', 'lock_version') end end end diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index ef8f8cbce3b..61c9fb5b4a4 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -211,7 +211,7 @@ RSpec.describe ApplicationExperiment, :experiment, feature_category: :experiment application_experiment.variant(:variant1) {} application_experiment.variant(:variant2) {} - expect(application_experiment.assigned.name).to eq('variant2') + expect(application_experiment.assigned.name).to eq(:variant2) end end @@ -248,7 +248,7 @@ RSpec.describe ApplicationExperiment, :experiment, feature_category: :experiment end it "caches the variant determined by the variant resolver" do - expect(application_experiment.assigned.name).to eq('candidate') # we should be in the experiment + expect(application_experiment.assigned.name).to eq(:candidate) # we should be in the experiment application_experiment.run @@ -263,7 +263,7 @@ RSpec.describe ApplicationExperiment, :experiment, feature_category: :experiment # the control. stub_feature_flags(namespaced_stub: false) # simulate being not rolled out - expect(application_experiment.assigned.name).to eq('control') # if we ask, it should be control + expect(application_experiment.assigned.name).to eq(:control) # if we ask, it should be control application_experiment.run @@ -299,29 +299,4 @@ RSpec.describe ApplicationExperiment, :experiment, feature_category: :experiment end end end - - context "with deprecation warnings" do - before do - Gitlab::Experiment::Configuration.instance_variable_set(:@__dep_versions, nil) # clear the internal memoization - - allow(ActiveSupport::Deprecation).to receive(:new).and_call_original - end - - it "doesn't warn on non dev/test environments" do - allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) - - expect { experiment(:example) { |e| e.use {} } }.not_to raise_error - expect(ActiveSupport::Deprecation).not_to have_received(:new).with(anything, 'Gitlab::Experiment') - end - - it "warns on dev and test environments" do - allow(Gitlab).to receive(:dev_or_test_env?).and_return(true) - - # This will eventually raise an ActiveSupport::Deprecation exception, - # it's ok to change it when that happens. - expect { experiment(:example) { |e| e.use {} } }.not_to raise_error - - expect(ActiveSupport::Deprecation).to have_received(:new).with(anything, 'Gitlab::Experiment') - end - end end diff --git a/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb b/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb index ee02fa5f1f2..9c6556745d7 100644 --- a/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb +++ b/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb @@ -4,6 +4,6 @@ require 'spec_helper' RSpec.describe SecurityReportsMrWidgetPromptExperiment do it "defines a control and candidate" do - expect(subject.behaviors.keys).to match_array(%w[control candidate]) + expect(subject.behaviors.keys).to match_array(%i[control candidate]) end end diff --git a/spec/features/issues/incident_issue_spec.rb b/spec/features/issues/incident_issue_spec.rb index 41bbd79202f..145b51d207a 100644 --- a/spec/features/issues/incident_issue_spec.rb +++ b/spec/features/issues/incident_issue_spec.rb @@ -51,8 +51,8 @@ RSpec.describe 'Incident Detail', :js, feature_category: :team_planning do aggregate_failures 'when on summary tab (default tab)' do hidden_items = find_all('.js-issue-widgets') - # Linked Issues/MRs and comment box and emoji block - expect(hidden_items.count).to eq(3) + # Description footer + Linked Issues/MRs + comment box + emoji block + expect(hidden_items.count).to eq(4) expect(hidden_items).to all(be_visible) edit_button = find_all('[aria-label="Edit title and description"]') diff --git a/spec/finders/abuse_reports_finder_spec.rb b/spec/finders/abuse_reports_finder_spec.rb index d3b148375d4..ee93d042ca2 100644 --- a/spec/finders/abuse_reports_finder_spec.rb +++ b/spec/finders/abuse_reports_finder_spec.rb @@ -78,6 +78,24 @@ RSpec.describe AbuseReportsFinder, '#execute' do expect(subject).to match_array([abuse_report_2]) end end + + context 'when value is not a valid status' do + let(:params) { { status: 'partial' } } + + it 'defaults to returning open abuse reports' do + expect(subject).to match_array([abuse_report_1]) + end + end + + context 'when abuse_reports_list feature flag is disabled' do + before do + stub_feature_flags(abuse_reports_list: false) + end + + it 'does not filter by status' do + expect(subject).to match_array([abuse_report_1, abuse_report_2]) + end + end end context 'when params[:category] is present' do diff --git a/spec/frontend/admin/abuse_reports/components/abuse_reports_filtered_search_bar_spec.js b/spec/frontend/admin/abuse_reports/components/abuse_reports_filtered_search_bar_spec.js index 9efab8403a0..990503c453d 100644 --- a/spec/frontend/admin/abuse_reports/components/abuse_reports_filtered_search_bar_spec.js +++ b/spec/frontend/admin/abuse_reports/components/abuse_reports_filtered_search_bar_spec.js @@ -58,21 +58,28 @@ describe('AbuseReportsFilteredSearchBar', () => { }); }); - it('sets status=open query when there is no initial status query', () => { - createComponent(); + it.each([undefined, 'invalid'])( + 'sets status=open query when initial status query is %s', + (status) => { + if (status) { + setWindowLocation(`?status=${status}`); + } - expect(updateHistory).toHaveBeenCalledWith({ - url: 'https://localhost/?status=open', - replace: true, - }); + createComponent(); - expect(findFilteredSearchBar().props('initialFilterValue')).toMatchObject([ - { - type: FILTERED_SEARCH_TOKEN_STATUS.type, - value: { data: 'open', operator: '=' }, - }, - ]); - }); + expect(updateHistory).toHaveBeenCalledWith({ + url: 'https://localhost/?status=open', + replace: true, + }); + + expect(findFilteredSearchBar().props('initialFilterValue')).toMatchObject([ + { + type: FILTERED_SEARCH_TOKEN_STATUS.type, + value: { data: 'open', operator: '=' }, + }, + ]); + }, + ); it('parses and passes search param to `FilteredSearchBar` component as `initialFilterValue` prop', () => { setWindowLocation('?status=closed&user=mr_abuser&reporter=ms_nitch'); diff --git a/spec/frontend/admin/abuse_reports/utils_spec.js b/spec/frontend/admin/abuse_reports/utils_spec.js index 17f0b9acb26..3908edd3fdd 100644 --- a/spec/frontend/admin/abuse_reports/utils_spec.js +++ b/spec/frontend/admin/abuse_reports/utils_spec.js @@ -1,5 +1,8 @@ -import { FILTERED_SEARCH_TOKEN_CATEGORY } from '~/admin/abuse_reports/constants'; -import { buildFilteredSearchCategoryToken } from '~/admin/abuse_reports/utils'; +import { + FILTERED_SEARCH_TOKEN_CATEGORY, + FILTERED_SEARCH_TOKEN_STATUS, +} from '~/admin/abuse_reports/constants'; +import { buildFilteredSearchCategoryToken, isValidStatus } from '~/admin/abuse_reports/utils'; describe('buildFilteredSearchCategoryToken', () => { it('adds correctly formatted options to FILTERED_SEARCH_TOKEN_CATEGORY', () => { @@ -11,3 +14,18 @@ describe('buildFilteredSearchCategoryToken', () => { }); }); }); + +describe('isValidStatus', () => { + const validStatuses = FILTERED_SEARCH_TOKEN_STATUS.options.map((o) => o.value); + + it.each(validStatuses)( + 'returns true when status is an option value of FILTERED_SEARCH_TOKEN_STATUS', + (status) => { + expect(isValidStatus(status)).toBe(true); + }, + ); + + it('return false when status is not an option value of FILTERED_SEARCH_TOKEN_STATUS', () => { + expect(isValidStatus('invalid')).toBe(false); + }); +}); diff --git a/spec/frontend/issues/show/components/app_spec.js b/spec/frontend/issues/show/components/app_spec.js index 1006f54eeaf..554662c611d 100644 --- a/spec/frontend/issues/show/components/app_spec.js +++ b/spec/frontend/issues/show/components/app_spec.js @@ -1,6 +1,5 @@ import { GlIcon, GlIntersectionObserver } from '@gitlab/ui'; import MockAdapter from 'axios-mock-adapter'; -import { setHTMLFixture } from 'helpers/fixtures'; import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; @@ -100,22 +99,6 @@ describe('Issuable output', () => { }; beforeEach(() => { - setHTMLFixture(` - <div> - <title>Title</title> - <div class="detail-page-description content-block"> - <details open> - <summary>One</summary> - </details> - <details> - <summary>Two</summary> - </details> - </div> - <div class="flash-container"></div> - <span id="task_status"></span> - </div> - `); - jest.spyOn(eventHub, '$emit'); axiosMock = new MockAdapter(axios); diff --git a/spec/frontend/issues/show/components/description_spec.js b/spec/frontend/issues/show/components/description_spec.js index 740b2f782e4..4b7b363f5da 100644 --- a/spec/frontend/issues/show/components/description_spec.js +++ b/spec/frontend/issues/show/components/description_spec.js @@ -1,8 +1,6 @@ -import $ from 'jquery'; import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import getIssueDetailsQuery from 'ee_else_ce/work_items/graphql/get_issue_details.query.graphql'; -import setWindowLocation from 'helpers/set_window_location_helper'; import { TEST_HOST } from 'helpers/test_constants'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; @@ -24,10 +22,6 @@ import { import { descriptionProps as initialProps, descriptionHtmlWithList } from '../mock_data/mock_data'; jest.mock('~/alert'); -jest.mock('~/lib/utils/url_utility', () => ({ - ...jest.requireActual('~/lib/utils/url_utility'), - updateHistory: jest.fn(), -})); jest.mock('~/task_list'); jest.mock('~/behaviors/markdown/render_gfm'); @@ -87,21 +81,6 @@ describe('Description component', () => { beforeEach(() => { window.gon = { sprite_icons: mockSpriteIcons }; - - setWindowLocation(TEST_HOST); - - if (!document.querySelector('.issuable-meta')) { - const metaData = document.createElement('div'); - metaData.classList.add('issuable-meta'); - metaData.innerHTML = - '<div class="flash-container"></div><span id="task_status"></span><span id="task_status_short"></span>'; - - document.body.appendChild(metaData); - } - }); - - afterAll(() => { - $('.issuable-meta .flash-container').remove(); }); it('doesnt animate first description changes', async () => { @@ -202,46 +181,6 @@ describe('Description component', () => { }); }); - describe('taskStatus', () => { - it('adds full taskStatus', async () => { - createComponent({ - props: { - taskStatus: '1 of 1', - }, - }); - await nextTick(); - - expect(document.querySelector('.issuable-meta #task_status').textContent.trim()).toBe( - '1 of 1', - ); - }); - - it('adds short taskStatus', async () => { - createComponent({ - props: { - taskStatus: '1 of 1', - }, - }); - await nextTick(); - - expect(document.querySelector('.issuable-meta #task_status_short').textContent.trim()).toBe( - '1/1 checklist item', - ); - }); - - it('clears task status text when no tasks are present', async () => { - createComponent({ - props: { - taskStatus: '0 of 0', - }, - }); - - await nextTick(); - - expect(document.querySelector('.issuable-meta #task_status').textContent.trim()).toBe(''); - }); - }); - describe('with list', () => { beforeEach(async () => { createComponent({ diff --git a/spec/frontend/issues/show/components/edited_spec.js b/spec/frontend/issues/show/components/edited_spec.js index a509627c347..dc0c7f5be46 100644 --- a/spec/frontend/issues/show/components/edited_spec.js +++ b/spec/frontend/issues/show/components/edited_spec.js @@ -1,41 +1,84 @@ +import { GlLink } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import { getTimeago } from '~/lib/utils/datetime_utility'; import Edited from '~/issues/show/components/edited.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; -const timeago = getTimeago(); - describe('Edited component', () => { let wrapper; - const findAuthorLink = () => wrapper.find('a'); + const timeago = getTimeago(); + const updatedAt = '2017-05-15T12:31:04.428Z'; + + const findAuthorLink = () => wrapper.findComponent(GlLink); const findTimeAgoTooltip = () => wrapper.findComponent(TimeAgoTooltip); const formatText = (text) => text.trim().replace(/\s\s+/g, ' '); const mountComponent = (propsData) => mount(Edited, { propsData }); - const updatedAt = '2017-05-15T12:31:04.428Z'; - it('renders an edited at+by string', () => { - wrapper = mountComponent({ - updatedAt, - updatedByName: 'Some User', - updatedByPath: '/some_user', + describe('task status section', () => { + describe('task status text', () => { + it('renders when there is a task status', () => { + wrapper = mountComponent({ taskCompletionStatus: { completed_count: 1, count: 3 } }); + + expect(wrapper.text()).toContain('1 of 3 checklist items completed'); + }); + + it('does not render when task count is 0', () => { + wrapper = mountComponent({ taskCompletionStatus: { completed_count: 0, count: 0 } }); + + expect(wrapper.text()).not.toContain('0 of 0 checklist items completed'); + }); }); - expect(formatText(wrapper.text())).toBe(`Edited ${timeago.format(updatedAt)} by Some User`); - expect(findAuthorLink().attributes('href')).toBe('/some_user'); - expect(findTimeAgoTooltip().exists()).toBe(true); + describe('checkmark', () => { + it('renders when all tasks are completed', () => { + wrapper = mountComponent({ taskCompletionStatus: { completed_count: 3, count: 3 } }); + + expect(wrapper.text()).toContain('✓'); + }); + + it('does not render when tasks are incomplete', () => { + wrapper = mountComponent({ taskCompletionStatus: { completed_count: 2, count: 3 } }); + + expect(wrapper.text()).not.toContain('✓'); + }); + + it('does not render when task count is 0', () => { + wrapper = mountComponent({ taskCompletionStatus: { completed_count: 0, count: 0 } }); + + expect(wrapper.text()).not.toContain('✓'); + }); + }); + + describe('middot', () => { + it('renders when there is also "Edited by" text', () => { + wrapper = mountComponent({ + taskCompletionStatus: { completed_count: 3, count: 3 }, + updatedAt, + }); + + expect(wrapper.text()).toContain('·'); + }); + + it('does not render when there is no "Edited by" text', () => { + wrapper = mountComponent({ taskCompletionStatus: { completed_count: 3, count: 3 } }); + + expect(wrapper.text()).not.toContain('·'); + }); + }); }); - it('if no updatedAt is provided, no time element will be rendered', () => { + it('renders an edited at+by string', () => { wrapper = mountComponent({ + updatedAt, updatedByName: 'Some User', updatedByPath: '/some_user', }); - expect(formatText(wrapper.text())).toBe('Edited by Some User'); + expect(formatText(wrapper.text())).toBe(`Edited ${timeago.format(updatedAt)} by Some User`); expect(findAuthorLink().attributes('href')).toBe('/some_user'); - expect(findTimeAgoTooltip().exists()).toBe(false); + expect(findTimeAgoTooltip().exists()).toBe(true); }); it('if no updatedByName and updatedByPath is provided, no user element will be rendered', () => { diff --git a/spec/frontend/issues/show/mock_data/mock_data.js b/spec/frontend/issues/show/mock_data/mock_data.js index 86d09665947..e923c0d97f4 100644 --- a/spec/frontend/issues/show/mock_data/mock_data.js +++ b/spec/frontend/issues/show/mock_data/mock_data.js @@ -5,7 +5,7 @@ export const initialRequest = { title_text: 'this is a title', description: '<p>this is a description!</p>', description_text: 'this is a description', - task_status: '2 of 4 completed', + task_completion_status: { completed_count: 2, count: 4 }, updated_at: '2015-05-15T12:31:04.428Z', updated_by_name: 'Some User', updated_by_path: '/some_user', @@ -17,7 +17,7 @@ export const secondRequest = { title_text: '2', description: '<p>42</p>', description_text: '42', - task_status: '0 of 0 completed', + task_completion_status: { completed_count: 0, count: 0 }, updated_at: '2016-05-15T12:31:04.428Z', updated_by_name: 'Other User', updated_by_path: '/other_user', @@ -30,7 +30,7 @@ export const putRequest = { title_text: 'PUT', description: '<p>PUT_DESC</p>', description_text: 'PUT_DESC', - task_status: '0 of 0 completed', + task_completion_status: { completed_count: 0, count: 0 }, updated_at: '2016-05-15T12:31:04.428Z', updated_by_name: 'Other User', updated_by_path: '/other_user', @@ -41,7 +41,6 @@ export const descriptionProps = { canUpdate: true, descriptionHtml: 'test', descriptionText: 'test', - taskStatus: '', updateUrl: TEST_HOST, }; @@ -60,6 +59,7 @@ export const appProps = { initialTitleText: '', initialDescriptionHtml: 'test', initialDescriptionText: 'test', + initialTaskCompletionStatus: { completed_count: 2, count: 4 }, lockVersion: 1, issueType: 'issue', markdownPreviewPath: '/', diff --git a/spec/frontend/work_items/components/notes/work_item_note_spec.js b/spec/frontend/work_items/components/notes/work_item_note_spec.js index 8e574dc1a81..2d6d12c69f9 100644 --- a/spec/frontend/work_items/components/notes/work_item_note_spec.js +++ b/spec/frontend/work_items/components/notes/work_item_note_spec.js @@ -178,8 +178,7 @@ describe('Work Item Note', () => { }, }); - expect(findEditedAt().exists()).toBe(true); - expect(findEditedAt().props()).toEqual({ + expect(findEditedAt().props()).toMatchObject({ updatedAt: '2023-02-12T07:47:40Z', updatedByName: 'Administrator', updatedByPath: 'test-path', diff --git a/spec/frontend/work_items/components/work_item_description_spec.js b/spec/frontend/work_items/components/work_item_description_spec.js index b4b7b8989ea..3482d011be9 100644 --- a/spec/frontend/work_items/components/work_item_description_spec.js +++ b/spec/frontend/work_items/components/work_item_description_spec.js @@ -179,7 +179,7 @@ describe('WorkItemDescription', () => { }), }); - expect(findEditedAt().props()).toEqual({ + expect(findEditedAt().props()).toMatchObject({ updatedAt: lastEditedAt, updatedByName: lastEditedBy.name, updatedByPath: lastEditedBy.webPath, diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index fd10b204e50..7549e37d4fd 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -416,7 +416,7 @@ RSpec.describe IssuablesHelper, feature_category: :team_planning do initialTitleText: issue.title, initialDescriptionHtml: '<p dir="auto">issue text</p>', initialDescriptionText: 'issue text', - initialTaskStatus: '0 of 0 checklist items completed', + initialTaskCompletionStatus: { completed_count: 0, count: 0 }, issueType: 'issue', iid: issue.iid.to_s, isHidden: false diff --git a/spec/lib/gitlab/experiment/rollout/feature_spec.rb b/spec/lib/gitlab/experiment/rollout/feature_spec.rb index a66f4fea207..cd46e7b3386 100644 --- a/spec/lib/gitlab/experiment/rollout/feature_spec.rb +++ b/spec/lib/gitlab/experiment/rollout/feature_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::Experiment::Rollout::Feature, :experiment do - subject { described_class.new.for(subject_experiment) } + subject { described_class.new(subject_experiment) } let(:subject_experiment) { experiment('namespaced/stub') } diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 05a41ddc6c5..7a31fd6a77d 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -136,7 +136,6 @@ RSpec.describe Notes::CreateService, feature_category: :team_planning do let(:action) { 'create_commit_comment' } let(:label) { 'counts.commit_comment' } let(:namespace) { project.namespace } - let(:feature_flag_name) { :route_hll_to_snowplow_phase4 } end end diff --git a/spec/workers/namespaces/root_statistics_worker_spec.rb b/spec/workers/namespaces/root_statistics_worker_spec.rb index edea9132850..bc2eca86711 100644 --- a/spec/workers/namespaces/root_statistics_worker_spec.rb +++ b/spec/workers/namespaces/root_statistics_worker_spec.rb @@ -3,48 +3,88 @@ require 'spec_helper' RSpec.describe Namespaces::RootStatisticsWorker, '#perform', feature_category: :source_code_management do - let(:group) { create(:group, :with_aggregation_schedule) } + let_it_be(:group) { create(:group, :with_aggregation_schedule) } subject(:worker) { described_class.new } - context 'with a namespace' do + RSpec.shared_examples 'bypasses aggregation schedule' do it 'executes refresher service' do expect_any_instance_of(Namespaces::StatisticsRefresherService) .to receive(:execute).and_call_original + expect(group).not_to receive(:aggregation_scheduled?) worker.perform(group.id) end - it 'deletes namespace aggregated schedule row' do - worker.perform(group.id) + it 'does not change AggregationSchedule count' do + expect do + worker.perform(group.id) + end.not_to change { Namespace::AggregationSchedule.count } + end + end + + context 'with a namespace' do + context 'with remove_aggregation_schedule_lease feature flag enabled' do + it_behaves_like 'bypasses aggregation schedule' + + context 'when something goes wrong when updating' do + before do + allow_any_instance_of(Namespaces::StatisticsRefresherService) + .to receive(:execute) + .and_raise(Namespaces::StatisticsRefresherService::RefresherError, 'error') + end + + it 'logs the error' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once - expect(group.reload.aggregation_schedule).to be_nil + worker.perform(group.id) + end + end end - context 'when something goes wrong when updating' do + context 'with remove_aggregation_schedule_lease feature flag disabled' do before do - allow_any_instance_of(Namespaces::StatisticsRefresherService) - .to receive(:execute) - .and_raise(Namespaces::StatisticsRefresherService::RefresherError, 'error') + stub_feature_flags(remove_aggregation_schedule_lease: false) + end + + it 'executes refresher service' do + expect_any_instance_of(Namespaces::StatisticsRefresherService) + .to receive(:execute).and_call_original + + worker.perform(group.id) end - it 'does not delete the aggregation schedule' do + it 'deletes namespace aggregated schedule row' do worker.perform(group.id) - expect(group.reload.aggregation_schedule).to be_present + expect(group.reload.aggregation_schedule).to be_nil end - it 'logs the error' do - # A Namespace::RootStatisticsWorker is scheduled when - # a Namespace::AggregationSchedule is created, so having - # create(:group, :with_aggregation_schedule), will execute - # another worker - allow_any_instance_of(Namespace::AggregationSchedule) - .to receive(:schedule_root_storage_statistics).and_return(nil) + context 'when something goes wrong when updating' do + before do + allow_any_instance_of(Namespaces::StatisticsRefresherService) + .to receive(:execute) + .and_raise(Namespaces::StatisticsRefresherService::RefresherError, 'error') + end - expect(Gitlab::ErrorTracking).to receive(:track_exception).once + it 'does not delete the aggregation schedule' do + worker.perform(group.id) - worker.perform(group.id) + expect(group.reload.aggregation_schedule).to be_present + end + + it 'logs the error' do + # A Namespace::RootStatisticsWorker is scheduled when + # a Namespace::AggregationSchedule is created, so having + # create(:group, :with_aggregation_schedule), will execute + # another worker + allow_any_instance_of(Namespace::AggregationSchedule) + .to receive(:schedule_root_storage_statistics).and_return(nil) + + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + worker.perform(group.id) + end end end end @@ -67,26 +107,42 @@ RSpec.describe Namespaces::RootStatisticsWorker, '#perform', feature_category: : group.aggregation_schedule.destroy! end - it 'does not execute the refresher service' do - expect_any_instance_of(Namespaces::StatisticsRefresherService) - .not_to receive(:execute) + context 'with remove_aggregation_schedule_lease feature flag disabled' do + before do + stub_feature_flags(remove_aggregation_schedule_lease: false) + end - worker.perform(group.id) + it 'does not execute the refresher service' do + expect_any_instance_of(Namespaces::StatisticsRefresherService) + .not_to receive(:execute) + + worker.perform(group.id) + end + end + + context 'with remove_aggregation_schedule_lease feature flag enabled' do + it_behaves_like 'bypasses aggregation schedule' end end it_behaves_like 'an idempotent worker' do let(:job_args) { [group.id] } - it 'deletes one aggregation schedule' do - # Make sure the group and it's aggregation schedule are created before - # counting - group + context 'with remove_aggregation_schedule_lease feature flag disabled' do + before do + stub_feature_flags(remove_aggregation_schedule_lease: false) + end - expect { worker.perform(*job_args) } - .to change { Namespace::AggregationSchedule.count }.by(-1) - expect { worker.perform(*job_args) } - .not_to change { Namespace::AggregationSchedule.count } + it 'deletes one aggregation schedule' do + # Make sure the group and it's aggregation schedule are created before + # counting + group + + expect { worker.perform(*job_args) } + .to change { Namespace::AggregationSchedule.count }.by(-1) + expect { worker.perform(*job_args) } + .not_to change { Namespace::AggregationSchedule.count } + end end end diff --git a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb index 69bd0f1ce47..249c143606f 100644 --- a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb +++ b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb @@ -3,32 +3,68 @@ require 'spec_helper' RSpec.describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_redis_shared_state, feature_category: :source_code_management do - let(:group) { create(:group) } + let_it_be(:group) { create(:group) } subject(:worker) { described_class.new } + RSpec.shared_examples 'schedule root statistic worker' do + it 'enqueues only RootStatisticsWorker' do + expect(Namespaces::RootStatisticsWorker).to receive(:perform_async).with(group.root_ancestor.id) + expect(Namespace::AggregationSchedule).not_to receive(:safe_find_or_create_by!) + .with(namespace_id: group.root_ancestor.id) + + worker.perform(group.id) + end + + it 'does not change AggregationSchedule count' do + expect do + worker.perform(group.root_ancestor.id) + end.not_to change { Namespace::AggregationSchedule.count } + end + end + context 'when group is the root ancestor' do - context 'when aggregation schedule exists' do - it 'does not create a new one' do - stub_aggregation_schedule_statistics + context 'with remove_aggregation_schedule_lease feature flag enabled' do + context 'when aggregation schedule does not exist' do + it_behaves_like "schedule root statistic worker" + end - Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id) + context 'when aggregation schedule does exist' do + before do + Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id) + end - expect do - worker.perform(group.id) - end.not_to change { Namespace::AggregationSchedule.count } + it_behaves_like "schedule root statistic worker" end end - context 'when aggregation schedule does not exist' do - it 'creates one' do - stub_aggregation_schedule_statistics + context 'with remove_aggregation_schedule_lease feature flag disabled' do + before do + stub_feature_flags(remove_aggregation_schedule_lease: false) + end + + context 'when aggregation schedule exists' do + it 'does not create a new one' do + stub_aggregation_schedule_statistics + + Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id) - expect do - worker.perform(group.id) - end.to change { Namespace::AggregationSchedule.count }.by(1) + expect do + worker.perform(group.id) + end.not_to change { Namespace::AggregationSchedule.count } + end + end + + context 'when aggregation schedule does not exist' do + it 'creates one' do + stub_aggregation_schedule_statistics - expect(group.aggregation_schedule).to be_present + expect do + worker.perform(group.id) + end.to change { Namespace::AggregationSchedule.count }.by(1) + + expect(group.aggregation_schedule).to be_present + end end end end @@ -37,12 +73,22 @@ RSpec.describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_ let(:parent_group) { create(:group) } let(:group) { create(:group, parent: parent_group) } - it 'creates an aggregation schedule for the root' do - stub_aggregation_schedule_statistics + context 'with remove_aggregation_schedule_lease feature flag enabled' do + it_behaves_like "schedule root statistic worker" + end - worker.perform(group.id) + context 'with remove_aggregation_schedule_lease feature flag disabled' do + before do + stub_feature_flags(remove_aggregation_schedule_lease: false) + end - expect(parent_group.aggregation_schedule).to be_present + it 'creates an aggregation schedule for the root' do + stub_aggregation_schedule_statistics + + worker.perform(group.id) + + expect(parent_group.aggregation_schedule).to be_present + end end end @@ -57,11 +103,17 @@ RSpec.describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_ it_behaves_like 'an idempotent worker' do let(:job_args) { [group.id] } - it 'creates a single aggregation schedule' do - expect { worker.perform(*job_args) } - .to change { Namespace::AggregationSchedule.count }.by(1) - expect { worker.perform(*job_args) } - .not_to change { Namespace::AggregationSchedule.count } + context 'with remove_aggregation_schedule_lease feature flag disabled' do + before do + stub_feature_flags(remove_aggregation_schedule_lease: false) + end + + it 'creates a single aggregation schedule' do + expect { worker.perform(*job_args) } + .to change { Namespace::AggregationSchedule.count }.by(1) + expect { worker.perform(*job_args) } + .not_to change { Namespace::AggregationSchedule.count } + end end end |