diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-02-04 15:17:40 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-02-04 15:17:40 +0300 |
commit | 9486811b62db7f35906bae75f912aa89804e721b (patch) | |
tree | 92da5045f0554bf89a60a96eb4ac1ffa96bfa427 | |
parent | aa7870a90b5925412a38dd6a27522f83517b917e (diff) |
Add latest changes from gitlab-org/gitlab@master
60 files changed, 690 insertions, 305 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index f5c15409bd9..e409227560d 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -b3039284a69e472a3272d2b60c9f4eb28489f438 +e0e146ec58d2515c2767f77d2860afc4bb7c1c73 diff --git a/app/assets/javascripts/cycle_analytics/components/metric_popover.vue b/app/assets/javascripts/analytics/shared/components/metric_popover.vue index 8d90e7b2392..8d90e7b2392 100644 --- a/app/assets/javascripts/cycle_analytics/components/metric_popover.vue +++ b/app/assets/javascripts/analytics/shared/components/metric_popover.vue diff --git a/app/assets/javascripts/analytics/shared/components/metric_tile.vue b/app/assets/javascripts/analytics/shared/components/metric_tile.vue new file mode 100644 index 00000000000..845a3386f6c --- /dev/null +++ b/app/assets/javascripts/analytics/shared/components/metric_tile.vue @@ -0,0 +1,51 @@ +<script> +import { GlSingleStat } from '@gitlab/ui/dist/charts'; +import { redirectTo } from '~/lib/utils/url_utility'; +import MetricPopover from './metric_popover.vue'; + +export default { + name: 'MetricTile', + components: { + GlSingleStat, + MetricPopover, + }, + props: { + metric: { + type: Object, + required: true, + }, + }, + computed: { + decimalPlaces() { + const parsedFloat = parseFloat(this.metric.value); + return Number.isNaN(parsedFloat) || Number.isInteger(parsedFloat) ? 0 : 1; + }, + hasLinks() { + return this.metric.links?.length && this.metric.links[0].url; + }, + }, + methods: { + clickHandler({ links }) { + if (this.hasLinks) { + redirectTo(links[0].url); + } + }, + }, +}; +</script> +<template> + <div v-bind="$attrs"> + <gl-single-stat + :id="metric.identifier" + :value="`${metric.value}`" + :title="metric.label" + :unit="metric.unit || ''" + :should-animate="true" + :animation-decimal-places="decimalPlaces" + :class="{ 'gl-hover-cursor-pointer': hasLinks }" + tabindex="0" + @click="clickHandler(metric)" + /> + <metric-popover :metric="metric" :target="metric.identifier" /> + </div> +</template> diff --git a/app/assets/javascripts/cycle_analytics/components/value_stream_metrics.vue b/app/assets/javascripts/analytics/shared/components/value_stream_metrics.vue index 1a3544e7677..1a3544e7677 100644 --- a/app/assets/javascripts/cycle_analytics/components/value_stream_metrics.vue +++ b/app/assets/javascripts/analytics/shared/components/value_stream_metrics.vue diff --git a/app/assets/javascripts/analytics/shared/constants.js b/app/assets/javascripts/analytics/shared/constants.js index c06bd34f86f..2ac144ceb5e 100644 --- a/app/assets/javascripts/analytics/shared/constants.js +++ b/app/assets/javascripts/analytics/shared/constants.js @@ -1,4 +1,5 @@ import { masks } from 'dateformat'; +import { s__ } from '~/locale'; export const DATE_RANGE_LIMIT = 180; export const OFFSET_DATE_BY_ONE = 1; @@ -11,3 +12,47 @@ export const dateFormats = { defaultDateTime: 'mmm d, yyyy h:MMtt', month: 'mmmm', }; + +// Some content is duplicated due to backward compatibility. +// It will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/350614 in 14.9 +export const METRICS_POPOVER_CONTENT = { + 'lead-time': { + description: s__('ValueStreamAnalytics|Median time from issue created to issue closed.'), + }, + lead_time: { + description: s__('ValueStreamAnalytics|Median time from issue created to issue closed.'), + }, + 'cycle-time': { + description: s__( + "ValueStreamAnalytics|Median time from the earliest commit of a linked issue's merge request to when that issue is closed.", + ), + }, + cycle_time: { + description: s__( + "ValueStreamAnalytics|Median time from the earliest commit of a linked issue's merge request to when that issue is closed.", + ), + }, + 'lead-time-for-changes': { + description: s__( + 'ValueStreamAnalytics|Median time between merge request merge and deployment to a production environment for all MRs deployed in the given time period.', + ), + }, + lead_time_for_changes: { + description: s__( + 'ValueStreamAnalytics|Median time between merge request merge and deployment to a production environment for all MRs deployed in the given time period.', + ), + }, + issues: { description: s__('ValueStreamAnalytics|Number of new issues created.') }, + 'new-issue': { description: s__('ValueStreamAnalytics|Number of new issues created.') }, + 'new-issues': { description: s__('ValueStreamAnalytics|Number of new issues created.') }, + deploys: { description: s__('ValueStreamAnalytics|Total number of deploys to production.') }, + 'deployment-frequency': { + description: s__('ValueStreamAnalytics|Average number of deployments to production per day.'), + }, + deployment_frequency: { + description: s__('ValueStreamAnalytics|Average number of deployments to production per day.'), + }, + commits: { + description: s__('ValueStreamAnalytics|Number of commits pushed to the default branch'), + }, +}; diff --git a/app/assets/javascripts/analytics/shared/utils.js b/app/assets/javascripts/analytics/shared/utils.js index f55ef99964e..dde429ab278 100644 --- a/app/assets/javascripts/analytics/shared/utils.js +++ b/app/assets/javascripts/analytics/shared/utils.js @@ -1,4 +1,6 @@ import dateFormat from 'dateformat'; +import { hideFlash } from '~/flash'; +import { slugify } from '~/lib/utils/text_utility'; import { urlQueryToFilter } from '~/vue_shared/components/filtered_search_bar/filtered_search_utils'; import { dateFormats } from './constants'; @@ -69,3 +71,28 @@ export const getDataZoomOption = ({ }; }); }; + +export const removeFlash = (type = 'alert') => { + const flashEl = document.querySelector(`.flash-${type}`); + if (flashEl) { + hideFlash(flashEl); + } +}; + +/** + * Prepares metric data to be rendered in the metric_card component + * + * @param {MetricData[]} data - The metric data to be rendered + * @param {Object} popoverContent - Key value pair of data to display in the popover + * @returns {TransformedMetricData[]} An array of metrics ready to render in the metric_card + */ +export const prepareTimeMetricsData = (data = [], popoverContent = {}) => + data.map(({ title: label, identifier, ...rest }) => { + const metricIdentifier = identifier || slugify(label); + return { + ...rest, + label, + identifier: metricIdentifier, + description: popoverContent[metricIdentifier]?.description || '', + }; + }); diff --git a/app/assets/javascripts/cycle_analytics/components/base.vue b/app/assets/javascripts/cycle_analytics/components/base.vue index 210e76115a2..3d7a34581b3 100644 --- a/app/assets/javascripts/cycle_analytics/components/base.vue +++ b/app/assets/javascripts/cycle_analytics/components/base.vue @@ -2,11 +2,11 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { mapActions, mapState, mapGetters } from 'vuex'; import { getCookie, setCookie } from '~/lib/utils/common_utils'; +import ValueStreamMetrics from '~/analytics/shared/components/value_stream_metrics.vue'; import { toYmd } from '~/analytics/shared/utils'; import PathNavigation from '~/cycle_analytics/components/path_navigation.vue'; import StageTable from '~/cycle_analytics/components/stage_table.vue'; import ValueStreamFilters from '~/cycle_analytics/components/value_stream_filters.vue'; -import ValueStreamMetrics from '~/cycle_analytics/components/value_stream_metrics.vue'; import UrlSync from '~/vue_shared/components/url_sync.vue'; import { __ } from '~/locale'; import { SUMMARY_METRICS_REQUEST, METRICS_REQUESTS } from '../constants'; diff --git a/app/assets/javascripts/cycle_analytics/components/metric_tile.vue b/app/assets/javascripts/cycle_analytics/components/metric_tile.vue index 845a3386f6c..a5c20b237b3 100644 --- a/app/assets/javascripts/cycle_analytics/components/metric_tile.vue +++ b/app/assets/javascripts/cycle_analytics/components/metric_tile.vue @@ -1,7 +1,7 @@ <script> import { GlSingleStat } from '@gitlab/ui/dist/charts'; import { redirectTo } from '~/lib/utils/url_utility'; -import MetricPopover from './metric_popover.vue'; +import MetricPopover from '~/analytics/shared/components/metric_popover.vue'; export default { name: 'MetricTile', diff --git a/app/assets/javascripts/cycle_analytics/constants.js b/app/assets/javascripts/cycle_analytics/constants.js index 911e729dfc6..f0b2bd9dc5b 100644 --- a/app/assets/javascripts/cycle_analytics/constants.js +++ b/app/assets/javascripts/cycle_analytics/constants.js @@ -36,50 +36,6 @@ export const OVERVIEW_METRICS = { RECENT_ACTIVITY: 'RECENT_ACTIVITY', }; -// Some content is duplicated due to backward compatibility. -// It will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/350614 in 14.9 -export const METRICS_POPOVER_CONTENT = { - 'lead-time': { - description: s__('ValueStreamAnalytics|Median time from issue created to issue closed.'), - }, - lead_time: { - description: s__('ValueStreamAnalytics|Median time from issue created to issue closed.'), - }, - 'cycle-time': { - description: s__( - "ValueStreamAnalytics|Median time from the earliest commit of a linked issue's merge request to when that issue is closed.", - ), - }, - cycle_time: { - description: s__( - "ValueStreamAnalytics|Median time from the earliest commit of a linked issue's merge request to when that issue is closed.", - ), - }, - 'lead-time-for-changes': { - description: s__( - 'ValueStreamAnalytics|Median time between merge request merge and deployment to a production environment for all MRs deployed in the given time period.', - ), - }, - lead_time_for_changes: { - description: s__( - 'ValueStreamAnalytics|Median time between merge request merge and deployment to a production environment for all MRs deployed in the given time period.', - ), - }, - issues: { description: s__('ValueStreamAnalytics|Number of new issues created.') }, - 'new-issue': { description: s__('ValueStreamAnalytics|Number of new issues created.') }, - 'new-issues': { description: s__('ValueStreamAnalytics|Number of new issues created.') }, - deploys: { description: s__('ValueStreamAnalytics|Total number of deploys to production.') }, - 'deployment-frequency': { - description: s__('ValueStreamAnalytics|Average number of deployments to production per day.'), - }, - deployment_frequency: { - description: s__('ValueStreamAnalytics|Average number of deployments to production per day.'), - }, - commits: { - description: s__('ValueStreamAnalytics|Number of commits pushed to the default branch'), - }, -}; - export const SUMMARY_METRICS_REQUEST = [ { endpoint: METRIC_TYPE_SUMMARY, name: __('recent activity'), request: getValueStreamMetrics }, ]; diff --git a/app/assets/javascripts/cycle_analytics/utils.js b/app/assets/javascripts/cycle_analytics/utils.js index 4d78ad814e0..428bb11b950 100644 --- a/app/assets/javascripts/cycle_analytics/utils.js +++ b/app/assets/javascripts/cycle_analytics/utils.js @@ -1,14 +1,5 @@ -import { hideFlash } from '~/flash'; import { parseSeconds } from '~/lib/utils/datetime_utility'; import { formatTimeAsSummary } from '~/lib/utils/datetime/date_format_utility'; -import { slugify } from '~/lib/utils/text_utility'; - -export const removeFlash = (type = 'alert') => { - const flashEl = document.querySelector(`.flash-${type}`); - if (flashEl) { - hideFlash(flashEl); - } -}; /** * Takes the stages and median data, combined with the selected stage, to build an @@ -85,25 +76,6 @@ export const filterStagesByHiddenStatus = (stages = [], isHidden = true) => * @property {String} unit - String representing the decimal point value, e.g '1.5' */ -/** - * Prepares metric data to be rendered in the metric_card component - * - * @param {MetricData[]} data - The metric data to be rendered - * @param {Object} popoverContent - Key value pair of data to display in the popover - * @returns {TransformedMetricData[]} An array of metrics ready to render in the metric_card - */ - -export const prepareTimeMetricsData = (data = [], popoverContent = {}) => - data.map(({ title: label, identifier, ...rest }) => { - const metricIdentifier = identifier || slugify(label); - return { - ...rest, - label, - identifier: metricIdentifier, - description: popoverContent[metricIdentifier]?.description || '', - }; - }); - const extractFeatures = (gon) => ({ cycleAnalyticsForGroups: Boolean(gon?.licensed_features?.cycleAnalyticsForGroups), }); diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index edff2e67b20..4c7b8e8f667 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -223,25 +223,31 @@ export default { <template> <div class="content js-line-expansion-content"> - <a - v-if="canExpandDown" - class="gl-mx-2 gl-cursor-pointer js-unfold-down gl-display-inline-block gl-py-4" + <button + type="button" + :disabled="!canExpandDown" + class="js-unfold-down gl-mx-2 gl-py-4 gl-cursor-pointer" @click="handleExpandLines(EXPAND_DOWN)" > <gl-icon :size="12" name="expand-down" /> <span>{{ $options.i18n.showMore }}</span> - </a> - <a class="gl-mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()"> + </button> + <button + type="button" + class="js-unfold-all gl-mx-2 gl-py-4 gl-cursor-pointer" + @click="handleExpandLines()" + > <gl-icon :size="12" name="expand" /> <span>{{ $options.i18n.showAll }}</span> - </a> - <a - v-if="canExpandUp" - class="gl-mx-2 gl-cursor-pointer js-unfold gl-display-inline-block gl-py-4" + </button> + <button + type="button" + :disabled="!canExpandUp" + class="js-unfold gl-mx-2 gl-py-4 gl-cursor-pointer" @click="handleExpandLines(EXPAND_UP)" > <gl-icon :size="12" name="expand-up" /> <span>{{ $options.i18n.showMore }}</span> - </a> + </button> </div> </template> diff --git a/app/assets/javascripts/google_tag_manager/index.js b/app/assets/javascripts/google_tag_manager/index.js index d6767001eed..55987ce64e6 100644 --- a/app/assets/javascripts/google_tag_manager/index.js +++ b/app/assets/javascripts/google_tag_manager/index.js @@ -55,16 +55,15 @@ const pushEvent = (event, args = {}) => { } }; -const pushEnhancedEcommerceEvent = (event, currencyCode, args = {}) => { +const pushEnhancedEcommerceEvent = (event, args = {}) => { if (!window.dataLayer) { return; } try { - window.dataLayer.push({ ecommerce: null }); + window.dataLayer.push({ ecommerce: null }); // Clear the previous ecommerce object window.dataLayer.push({ event, - currencyCode, ...args, }); } catch (e) { @@ -189,6 +188,7 @@ export const trackCheckout = (selectedPlan, quantity) => { const eventData = { ecommerce: { + currencyCode: 'USD', checkout: { actionField: { step: 1 }, products: [product], @@ -197,7 +197,7 @@ export const trackCheckout = (selectedPlan, quantity) => { }; // eslint-disable-next-line @gitlab/require-i18n-strings - pushEnhancedEcommerceEvent('EECCheckout', 'USD', eventData); + pushEnhancedEcommerceEvent('EECCheckout', eventData); }; export const trackTransaction = (transactionDetails) => { @@ -215,6 +215,7 @@ export const trackTransaction = (transactionDetails) => { const eventData = { ecommerce: { + currencyCode: 'USD', purchase: { actionField: { id: transactionId, @@ -228,5 +229,5 @@ export const trackTransaction = (transactionDetails) => { }, }; - pushEnhancedEcommerceEvent('EECtransactionSuccess', 'USD', eventData); + pushEnhancedEcommerceEvent('EECtransactionSuccess', eventData); }; diff --git a/app/assets/javascripts/terraform/components/states_table.vue b/app/assets/javascripts/terraform/components/states_table.vue index d3c373e7db1..efc2991f40f 100644 --- a/app/assets/javascripts/terraform/components/states_table.vue +++ b/app/assets/javascripts/terraform/components/states_table.vue @@ -1,14 +1,5 @@ <script> -import { - GlAlert, - GlBadge, - GlIcon, - GlLink, - GlLoadingIcon, - GlSprintf, - GlTable, - GlTooltip, -} from '@gitlab/ui'; +import { GlAlert, GlBadge, GlLink, GlLoadingIcon, GlSprintf, GlTable, GlTooltip } from '@gitlab/ui'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { s__ } from '~/locale'; import CiBadge from '~/vue_shared/components/ci_badge_link.vue'; @@ -21,7 +12,6 @@ export default { CiBadge, GlAlert, GlBadge, - GlIcon, GlLink, GlLoadingIcon, GlSprintf, @@ -156,8 +146,7 @@ export default { :id="`terraformLockedBadgeContainer${item.name}`" class="gl-mx-3" > - <gl-badge :id="`terraformLockedBadge${item.name}`"> - <gl-icon name="lock" /> + <gl-badge :id="`terraformLockedBadge${item.name}`" icon="lock"> {{ $options.i18n.locked }} </gl-badge> diff --git a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_item.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_item.vue index af0235bfc69..138284c9f6b 100644 --- a/app/assets/javascripts/vue_shared/issuable/list/components/issuable_item.vue +++ b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_item.vue @@ -121,7 +121,10 @@ export default { }, showIssuableMeta() { return Boolean( - this.hasSlotContents('status') || this.showDiscussions || this.issuable.assignees, + this.hasSlotContents('status') || + this.hasSlotContents('statistics') || + this.showDiscussions || + this.issuable.assignees, ); }, issuableNotesLink() { diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss index 1f4221e5778..a3d0e2a04f9 100644 --- a/app/assets/stylesheets/framework/diffs.scss +++ b/app/assets/stylesheets/framework/diffs.scss @@ -582,6 +582,25 @@ table.code { } } +.diff-expansion-cell { + flex: 1 1; + min-width: max-content; +} + +.diff-expansion-cell-middle { + flex: 0 1 max-content; +} + +@media only screen and (min-width: $breakpoint-xl) { + .diff-expansion-cell-start { + text-align: right; + } + + .diff-expansion-cell-end { + text-align: left; + } +} + // Merge request diff grid layout .diff-grid { .diff-td { diff --git a/app/assets/stylesheets/highlight/common.scss b/app/assets/stylesheets/highlight/common.scss index 97dd7edef13..e4a8893a25e 100644 --- a/app/assets/stylesheets/highlight/common.scss +++ b/app/assets/stylesheets/highlight/common.scss @@ -28,8 +28,24 @@ border-bottom: 1px solid $border; } - a { + button { color: $link; + border: 0; + background: transparent; + + &[disabled] { + color: desaturate($link, 100%); + opacity: 0.5; + cursor: default; + } + + &:hover:not([disabled]) { + text-decoration: underline; + } + + &:not(:focus-visible) { + outline: 0; + } } } diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index cdef843c9b4..ad05cbd1f0e 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -890,3 +890,13 @@ } } } + +.icon-overlap-and-shadow { + filter: + drop-shadow(0 1px 0.5px #fff) + drop-shadow(1px 0 0.5px #fff) + drop-shadow(0 -1px 0.5px #fff) + drop-shadow(-1px 0 0.5px #fff); + margin-right: -7px; + z-index: 1; +} diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index ddf70c1892a..d1c409d071e 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -3,6 +3,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController include Gitlab::Experimentation::ControllerConcern include InitializesCurrentUserMode + include Gitlab::Utils::StrongMemoize before_action :verify_confirmed_email!, :verify_confidential_application! @@ -27,6 +28,56 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController private + def pre_auth_params + # Cannot be achieved with a before_action hook, due to the execution order. + downgrade_scopes! if action_name == 'new' + + super + end + + # limit scopes when signing in with GitLab + def downgrade_scopes! + return unless Feature.enabled?(:omniauth_login_minimal_scopes, current_user, + default_enabled: :yaml) + + auth_type = params.delete('gl_auth_type') + return unless auth_type == 'login' + + ensure_read_user_scope! + + params['scope'] = Gitlab::Auth::READ_USER_SCOPE.to_s if application_has_read_user_scope? + end + + # Configure the application to support read_user scope, if it already + # supports scopes with greater levels of privileges. + def ensure_read_user_scope! + return if application_has_read_user_scope? + return unless application_has_api_scope? + + add_read_user_scope! + end + + def add_read_user_scope! + return unless doorkeeper_application + + scopes = doorkeeper_application.scopes + scopes.add(Gitlab::Auth::READ_USER_SCOPE) + doorkeeper_application.scopes = scopes + doorkeeper_application.save! + end + + def doorkeeper_application + strong_memoize(:doorkeeper_application) { ::Doorkeeper::OAuth::Client.find(params['client_id'])&.application } + end + + def application_has_read_user_scope? + doorkeeper_application&.includes_scope?(Gitlab::Auth::READ_USER_SCOPE) + end + + def application_has_api_scope? + doorkeeper_application&.includes_scope?(*::Gitlab::Auth::API_SCOPES) + end + # Confidential apps require the client_secret to be sent with the request. # Doorkeeper allows implicit grant flow requests (response_type=token) to # work without client_secret regardless of the confidential setting. diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index c9966d87b68..b39cd485fe8 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -62,10 +62,10 @@ module ProjectsHelper name: author.name } - inject_classes = ["author-link"] + inject_classes = ["author-link", opts[:extra_class]] if opts[:name] - inject_classes.concat(["js-user-link", opts[:extra_class], opts[:mobile_classes]]) + inject_classes.concat(["js-user-link", opts[:mobile_classes]]) else inject_classes.append( "has-tooltip" ) end diff --git a/app/models/namespaces/sync_event.rb b/app/models/namespaces/sync_event.rb index 8534d8afb8c..fbe047f2c5a 100644 --- a/app/models/namespaces/sync_event.rb +++ b/app/models/namespaces/sync_event.rb @@ -13,4 +13,8 @@ class Namespaces::SyncEvent < ApplicationRecord def self.enqueue_worker ::Namespaces::ProcessSyncEventsWorker.perform_async # rubocop:disable CodeReuse/Worker end + + def self.upper_bound_count + select('COALESCE(MAX(id) - MIN(id) + 1, 0) AS upper_bound_count').to_a.first.upper_bound_count + end end diff --git a/app/models/projects/sync_event.rb b/app/models/projects/sync_event.rb index 5221b00c55f..7af863c0cf0 100644 --- a/app/models/projects/sync_event.rb +++ b/app/models/projects/sync_event.rb @@ -13,4 +13,8 @@ class Projects::SyncEvent < ApplicationRecord def self.enqueue_worker ::Projects::ProcessSyncEventsWorker.perform_async # rubocop:disable CodeReuse/Worker end + + def self.upper_bound_count + select('COALESCE(MAX(id) - MIN(id) + 1, 0) AS upper_bound_count').to_a.first.upper_bound_count + end end diff --git a/app/services/ci/process_sync_events_service.rb b/app/services/ci/process_sync_events_service.rb index 184dd9c2c8a..d90ee02b1c6 100644 --- a/app/services/ci/process_sync_events_service.rb +++ b/app/services/ci/process_sync_events_service.rb @@ -2,7 +2,6 @@ module Ci class ProcessSyncEventsService - include Gitlab::Utils::StrongMemoize include ExclusiveLeaseGuard BATCH_SIZE = 1000 @@ -10,6 +9,7 @@ module Ci def initialize(sync_event_class, sync_class) @sync_event_class = sync_event_class @sync_class = sync_class + @results = {} end def execute @@ -17,13 +17,19 @@ module Ci try_obtain_lease { process_events } enqueue_worker_if_there_still_event + + @results end private def process_events + add_result(estimated_total_events: @sync_event_class.upper_bound_count) + events = @sync_event_class.preload_synced_relation.first(BATCH_SIZE) + add_result(consumable_events: events.size) + return if events.empty? processed_events = [] @@ -35,6 +41,7 @@ module Ci processed_events << event end ensure + add_result(processed_events: processed_events.size) @sync_event_class.id_in(processed_events).delete_all end end @@ -50,5 +57,9 @@ module Ci def lease_timeout 1.minute end + + def add_result(result) + @results.merge!(result) + end end end diff --git a/app/views/groups/settings/_general.html.haml b/app/views/groups/settings/_general.html.haml index 30dd6a28f61..dfb7932a781 100644 --- a/app/views/groups/settings/_general.html.haml +++ b/app/views/groups/settings/_general.html.haml @@ -30,7 +30,7 @@ = render 'shared/choose_avatar_button', f: f - if @group.avatar? %hr - = link_to s_('Groups|Remove avatar'), group_avatar_path(@group.to_param), data: { confirm: s_('Groups|Avatar will be removed. Are you sure?')}, method: :delete, class: 'gl-button btn btn-danger-secondary' + = link_to s_('Groups|Remove avatar'), group_avatar_path(@group.to_param), aria: { label: s_('Groups|Remove avatar') }, data: { confirm: s_('Groups|Avatar will be removed. Are you sure?'), 'confirm-btn-variant': 'danger' }, method: :delete, class: 'gl-button btn btn-danger-secondary' = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group = f.submit s_('Groups|Save changes'), class: 'btn gl-button btn-confirm mt-4 js-dirty-submit', data: { qa_selector: 'save_name_visibility_settings_button' } diff --git a/app/views/shared/access_tokens/_table.html.haml b/app/views/shared/access_tokens/_table.html.haml index aa579b4a672..7f7dafbe5b0 100644 --- a/app/views/shared/access_tokens/_table.html.haml +++ b/app/views/shared/access_tokens/_table.html.haml @@ -56,7 +56,7 @@ %span.token-never-expires-label= _('Never') - if resource %td= resource.member(token.user).human_access - %td= link_to _('Revoke'), revoke_route_helper.call(token), method: :put, class: "gl-button btn btn-danger btn-sm float-right qa-revoke-button #{'btn-danger-secondary' unless token.expires?}", data: { confirm: _('Are you sure you want to revoke this %{type}? This action cannot be undone.') % { type: type } } + %td= link_to _('Revoke'), revoke_route_helper.call(token), method: :put, class: "gl-button btn btn-danger btn-sm float-right qa-revoke-button #{'btn-danger-secondary' unless token.expires?}", aria: { label: _('Revoke') }, data: { confirm: _('Are you sure you want to revoke this %{type}? This action cannot be undone.') % { type: type }, 'confirm-btn-variant': 'danger' } - else .settings-message.text-center = no_active_tokens_message diff --git a/app/views/shared/issuable/_assignees.html.haml b/app/views/shared/issuable/_assignees.html.haml index 196d0417fb8..e6d722cb08d 100644 --- a/app/views/shared/issuable/_assignees.html.haml +++ b/app/views/shared/issuable/_assignees.html.haml @@ -3,8 +3,11 @@ - render_count = assignees_rendering_overflow ? max_render - 1 : max_render - more_assignees_count = issuable.assignees.size - render_count -- issuable.assignees.take(render_count).each do |assignee| # rubocop: disable CodeReuse/ActiveRecord - = link_to_member(@project, assignee, name: false, title: _("Assigned to %{name}") % { name: assignee.name}) +- if issuable.instance_of?(MergeRequest) && Feature.enabled?(:mr_attention_requests, default_enabled: :yaml) + = render 'shared/issuable/merge_request_assignees', issuable: issuable, count: render_count +- else + - issuable.assignees.take(render_count).each do |assignee| # rubocop: disable CodeReuse/ActiveRecord + = link_to_member(@project, assignee, name: false, title: s_("MrList|Assigned to %{name}, go to their profile.") % { name: assignee.name}) - if more_assignees_count > 0 %span{ class: 'avatar-counter has-tooltip', data: { container: 'body', placement: 'bottom', 'line-type' => 'old', qa_selector: 'avatar_counter_content' }, title: _("+%{more_assignees_count} more assignees") % { more_assignees_count: more_assignees_count} } diff --git a/app/views/shared/issuable/_merge_request_assignees.html.haml b/app/views/shared/issuable/_merge_request_assignees.html.haml new file mode 100644 index 00000000000..13dc6ae4abb --- /dev/null +++ b/app/views/shared/issuable/_merge_request_assignees.html.haml @@ -0,0 +1,8 @@ +- issuable.merge_request_assignees.take(count).each do |merge_request_assignee| # rubocop: disable CodeReuse/ActiveRecord + - assignee = merge_request_assignee.assignee + - assignee_tooltip = ( merge_request_assignee.attention_requested? ? s_("MrList|Attention requested from assignee %{name}, go to their profile.") : s_("MrList|Assigned to %{name}, go to their profile.") ) % { name: assignee.name} + + = link_to_member(@project, assignee, name: false, title: assignee_tooltip, extra_class: "gl-flex-direction-row-reverse") do + - if merge_request_assignee.attention_requested? + %span.gl-display-inline-flex + = sprite_icon('attention-solid-sm', css_class: 'gl-text-orange-500 icon-overlap-and-shadow') diff --git a/app/views/shared/issuable/_merge_request_reviewers.html.haml b/app/views/shared/issuable/_merge_request_reviewers.html.haml new file mode 100644 index 00000000000..df5c69e309f --- /dev/null +++ b/app/views/shared/issuable/_merge_request_reviewers.html.haml @@ -0,0 +1,8 @@ +- issuable.merge_request_reviewers.take(count).each do |merge_request_reviewer| # rubocop: disable CodeReuse/ActiveRecord + - reviewer = merge_request_reviewer.reviewer + - reviewer_tooltip = ( merge_request_reviewer.attention_requested? ? s_("MrList|Attention requested from reviewer %{name}, go to their profile.") : s_("MrList|Review requested from %{name}, go to their profile.") ) % { name: reviewer.name} + + = link_to_member(@project, reviewer, name: false, title: reviewer_tooltip, extra_class: "gl-flex-direction-row-reverse") do + - if merge_request_reviewer.attention_requested? + %span.gl-display-inline-flex + = sprite_icon('attention-solid-sm', css_class: 'gl-text-orange-500 icon-overlap-and-shadow') diff --git a/app/views/shared/issuable/_reviewers.html.haml b/app/views/shared/issuable/_reviewers.html.haml index 8e66135a20b..0bb0faa0bb8 100644 --- a/app/views/shared/issuable/_reviewers.html.haml +++ b/app/views/shared/issuable/_reviewers.html.haml @@ -3,8 +3,11 @@ - render_count = reviewers_rendering_overflow ? max_render - 1 : max_render - more_reviewers_count = issuable.reviewers.size - render_count -- issuable.reviewers.take(render_count).each do |reviewer| # rubocop: disable CodeReuse/ActiveRecord - = link_to_member(@project, reviewer, name: false, title: _("Review requested from %{name}") % { name: reviewer.name}) +- if issuable.instance_of?(MergeRequest) && Feature.enabled?(:mr_attention_requests, default_enabled: :yaml) + = render 'shared/issuable/merge_request_reviewers', issuable: issuable, count: render_count +- else + - issuable.reviewers.take(render_count).each do |reviewer| # rubocop: disable CodeReuse/ActiveRecord + = link_to_member(@project, reviewer, name: false, title: s_("MrList|Review requested from %{name}, go to their profile.") % { name: reviewer.name}) - if more_reviewers_count > 0 %span{ class: 'avatar-counter has-tooltip', data: { container: 'body', placement: 'bottom', 'line-type' => 'old' }, title: _("+%{more_reviewers_count} more reviewers") % { more_reviewers_count: more_reviewers_count} } diff --git a/app/workers/namespaces/process_sync_events_worker.rb b/app/workers/namespaces/process_sync_events_worker.rb index f3c4f5bebb1..269710dd804 100644 --- a/app/workers/namespaces/process_sync_events_worker.rb +++ b/app/workers/namespaces/process_sync_events_worker.rb @@ -16,7 +16,13 @@ module Namespaces deduplicate :until_executing def perform - ::Ci::ProcessSyncEventsService.new(::Namespaces::SyncEvent, ::Ci::NamespaceMirror).execute + results = ::Ci::ProcessSyncEventsService.new( + ::Namespaces::SyncEvent, ::Ci::NamespaceMirror + ).execute + + results.each do |key, value| + log_extra_metadata_on_done(key, value) + end end end end diff --git a/app/workers/projects/process_sync_events_worker.rb b/app/workers/projects/process_sync_events_worker.rb index b7c4b4de3d0..1330ae47a68 100644 --- a/app/workers/projects/process_sync_events_worker.rb +++ b/app/workers/projects/process_sync_events_worker.rb @@ -16,7 +16,13 @@ module Projects deduplicate :until_executing def perform - ::Ci::ProcessSyncEventsService.new(::Projects::SyncEvent, ::Ci::ProjectMirror).execute + results = ::Ci::ProcessSyncEventsService.new( + ::Projects::SyncEvent, ::Ci::ProjectMirror + ).execute + + results.each do |key, value| + log_extra_metadata_on_done(key, value) + end end end end diff --git a/config/feature_flags/development/omniauth_login_minimal_scopes.yml b/config/feature_flags/development/omniauth_login_minimal_scopes.yml new file mode 100644 index 00000000000..b2ca3484a98 --- /dev/null +++ b/config/feature_flags/development/omniauth_login_minimal_scopes.yml @@ -0,0 +1,8 @@ +--- +name: omniauth_login_minimal_scopes +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78556 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351331 +milestone: '14.8' +type: development +group: 'group::authentication and authorization' +default_enabled: false diff --git a/doc/integration/gitlab.md b/doc/integration/gitlab.md index 2dd357e50a6..74ae9bb1998 100644 --- a/doc/integration/gitlab.md +++ b/doc/integration/gitlab.md @@ -24,7 +24,12 @@ GitLab.com generates an application ID and secret key for you to use. http://your-gitlab.example.com/users/auth/gitlab/callback ``` - The first link is required for the importer and second for the authorization. + The first link is required for the importer and second for authentication. + + If you: + + - Plan to use the importer, you can leave scopes as they are. + - Only want to use this application for authentication, we recommend using a more minimal set of scopes. `read_user` is sufficient. 1. Select **Save application**. 1. You should now see an **Application ID** and **Secret**. Keep this page open as you continue @@ -57,7 +62,9 @@ GitLab.com generates an application ID and secret key for you to use. # label: "Provider name", # optional label for login button, defaults to "GitLab.com" app_id: "YOUR_APP_ID", app_secret: "YOUR_APP_SECRET", - args: { scope: "api" } + args: { scope: "read_user" # optional: defaults to the scopes of the application + , client_options: { site: "https://gitlab.example.com/api/v4" } + } } ] ``` @@ -71,7 +78,8 @@ GitLab.com generates an application ID and secret key for you to use. label: "Provider name", # optional label for login button, defaults to "GitLab.com" app_id: "YOUR_APP_ID", app_secret: "YOUR_APP_SECRET", - args: { scope: "api", client_options: { site: "https://gitlab.example.com/api/v4" } } + args: { scope: "read_user" # optional: defaults to the scopes of the application + , client_options: { site: "https://gitlab.example.com/api/v4" } } } ] ``` @@ -83,7 +91,7 @@ GitLab.com generates an application ID and secret key for you to use. # label: 'Provider name', # optional label for login button, defaults to "GitLab.com" app_id: 'YOUR_APP_ID', app_secret: 'YOUR_APP_SECRET', - args: { scope: 'api' } } + args: { "client_options": { "site": 'https://gitlab.example.com/api/v4' } } ``` Or, for installations from source to authenticate against a different GitLab instance: @@ -93,7 +101,7 @@ GitLab.com generates an application ID and secret key for you to use. label: 'Provider name', # optional label for login button, defaults to "GitLab.com" app_id: 'YOUR_APP_ID', app_secret: 'YOUR_APP_SECRET', - args: { scope: 'api', "client_options": { "site": 'https://gitlab.example.com/api/v4' } } + args: { "client_options": { "site": 'https://gitlab.example.com/api/v4' } } ``` 1. Change `'YOUR_APP_ID'` to the Application ID from the GitLab.com application page. @@ -101,7 +109,6 @@ GitLab.com generates an application ID and secret key for you to use. 1. Save the configuration file. 1. Based on how GitLab was installed, implement these changes by using the appropriate method: - - Omnibus GitLab: [reconfigure GitLab](../administration/restart_gitlab.md#omnibus-gitlab-reconfigure). - Source: [restart GitLab](../administration/restart_gitlab.md#installations-from-source). @@ -110,3 +117,20 @@ regular sign-in form. Select the icon to begin the authentication process. GitLab.com asks the user to sign in and authorize the GitLab application. If everything goes well, the user is returned to your GitLab instance and is signed in. + +## Reduce access privileges on sign in + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/337663) in GitLab 14.8 [with a flag](../administration/feature_flags.md) named `omniauth_login_minimal_scopes`. Disabled by default. + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../administration/feature_flags.md) named `omniauth_login_minimal_scopes`. On GitLab.com, this feature is not available. + +If you use a GitLab instance for authentication, you can reduce access rights when an OAuth application is used for sign in. + +Any OAuth application can advertise the purpose of the application with the +authorization parameter: `gl_auth_type=login`. If the application is +configured with `api` or `read_api`, the access token is issued with +`read_user` for login, because no higher permissions are needed. + +The GitLab OAuth client is configured to pass this parameter, but other +applications can also pass it. diff --git a/doc/integration/openid_connect_provider.md b/doc/integration/openid_connect_provider.md index e85231d1c25..1254b29bfb9 100644 --- a/doc/integration/openid_connect_provider.md +++ b/doc/integration/openid_connect_provider.md @@ -35,12 +35,15 @@ is select the `openid` scope in the application settings. ## Settings discovery -If your client allows importing OIDC settings from a discovery URL, you can use the following URL to automatically find the correct settings: +If your client allows importing OIDC settings from a discovery URL, you can use +the following URL to automatically find the correct settings for GitLab.com: ```plaintext -https://gitlab.example.com/.well-known/openid-configuration +https://gitlab.com/.well-known/openid-configuration ``` +Similar URLs can be used for other GitLab instances. + ## Shared information The following user information is shared with clients: diff --git a/lib/api/scope.rb b/lib/api/scope.rb index 707775e5d15..62aefcceb4b 100644 --- a/lib/api/scope.rb +++ b/lib/api/scope.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true # Encapsulate a scope used for authorization, such as `api`, or `read_user` +# See Gitlab::Auth for the set of available scopes, and their purposes. module API class Scope attr_reader :name, :if diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 257c73c47e6..5d5a431f206 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -6,25 +6,35 @@ module Gitlab IpBlacklisted = Class.new(StandardError) # Scopes used for GitLab API access - API_SCOPES = [:api, :read_user, :read_api].freeze + API_SCOPE = :api + READ_API_SCOPE = :read_api + READ_USER_SCOPE = :read_user + API_SCOPES = [API_SCOPE, READ_API_SCOPE, READ_USER_SCOPE].freeze + + PROFILE_SCOPE = :profile + EMAIL_SCOPE = :email + OPENID_SCOPE = :openid + # Scopes used for OpenID Connect + OPENID_SCOPES = [OPENID_SCOPE].freeze + # OpenID Connect profile scopes + PROFILE_SCOPES = [PROFILE_SCOPE, EMAIL_SCOPE].freeze # Scopes used for GitLab Repository access - REPOSITORY_SCOPES = [:read_repository, :write_repository].freeze + READ_REPOSITORY_SCOPE = :read_repository + WRITE_REPOSITORY_SCOPE = :write_repository + REPOSITORY_SCOPES = [READ_REPOSITORY_SCOPE, WRITE_REPOSITORY_SCOPE].freeze # Scopes used for GitLab Docker Registry access - REGISTRY_SCOPES = [:read_registry, :write_registry].freeze + READ_REGISTRY_SCOPE = :read_registry + WRITE_REGISTRY_SCOPE = :write_registry + REGISTRY_SCOPES = [READ_REGISTRY_SCOPE, WRITE_REGISTRY_SCOPE].freeze # Scopes used for GitLab as admin - ADMIN_SCOPES = [:sudo].freeze - - # Scopes used for OpenID Connect - OPENID_SCOPES = [:openid].freeze - - # OpenID Connect profile scopes - PROFILE_SCOPES = [:profile, :email].freeze + SUDO_SCOPE = :sudo + ADMIN_SCOPES = [SUDO_SCOPE].freeze # Default scopes for OAuth applications that don't define their own - DEFAULT_SCOPES = [:api].freeze + DEFAULT_SCOPES = [API_SCOPE].freeze CI_JOB_USER = 'gitlab-ci-token' diff --git a/lib/gitlab/omniauth_initializer.rb b/lib/gitlab/omniauth_initializer.rb index 3e14e1789bb..a9ff186c7cb 100644 --- a/lib/gitlab/omniauth_initializer.rb +++ b/lib/gitlab/omniauth_initializer.rb @@ -28,6 +28,10 @@ module Gitlab { fail_with_empty_uid: true } when 'google_oauth2' { client_options: { connection_opts: { request: { timeout: OAUTH2_TIMEOUT_SECONDS } } } } + when 'gitlab' + { + authorize_params: { gl_auth_type: 'login' } + } else {} end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3b828f28cd1..cbc023e5b46 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4918,9 +4918,6 @@ msgstr "" msgid "Assigned to %{assignee_name}" msgstr "" -msgid "Assigned to %{name}" -msgstr "" - msgid "Assigned to me" msgstr "" @@ -23494,6 +23491,18 @@ msgstr "" msgid "MrDeploymentActions|Stop environment" msgstr "" +msgid "MrList|Assigned to %{name}, go to their profile." +msgstr "" + +msgid "MrList|Attention requested from assignee %{name}, go to their profile." +msgstr "" + +msgid "MrList|Attention requested from reviewer %{name}, go to their profile." +msgstr "" + +msgid "MrList|Review requested from %{name}, go to their profile." +msgstr "" + msgid "Multi-project" msgstr "" @@ -30777,9 +30786,6 @@ msgstr "" msgid "Review changes" msgstr "" -msgid "Review requested from %{name}" -msgstr "" - msgid "Review requests for you" msgstr "" @@ -52,7 +52,8 @@ module QA "otp" => "OTP", "jira_api" => "JiraAPI", "registry_tls" => "RegistryTLS", - "jetbrains" => "JetBrains" + "jetbrains" => "JetBrains", + "vscode" => "VSCode" ) loader.setup diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 98cc8d83e0c..e6553c027d6 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -4,7 +4,13 @@ require 'spec_helper' RSpec.describe Oauth::AuthorizationsController do let(:user) { create(:user) } - let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') } + let(:application_scopes) { 'api read_user' } + + let!(:application) do + create(:oauth_application, scopes: application_scopes, + redirect_uri: 'http://example.com') + end + let(:params) do { response_type: "code", @@ -119,6 +125,92 @@ RSpec.describe Oauth::AuthorizationsController do expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template('doorkeeper/authorizations/redirect') end + + context 'with gl_auth_type=login' do + let(:minimal_scope) { Gitlab::Auth::READ_USER_SCOPE.to_s } + + before do + params[:gl_auth_type] = 'login' + end + + shared_examples 'downgrades scopes' do + it 'downgrades the scopes' do + subject + + pre_auth = controller.send(:pre_auth) + + expect(pre_auth.scopes).to contain_exactly(minimal_scope) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/new') + # See: config/locales/doorkeeper.en.yml + expect(response.body).to include("Read the authenticated user's personal information") + expect(response.body).not_to include("Access the authenticated user's API") + end + end + + shared_examples 'adds read_user scope' do + it 'modifies the client.application.scopes' do + expect { subject } + .to change { application.reload.scopes }.to include(minimal_scope) + end + + it 'does not remove pre-existing scopes' do + subject + + expect(application.scopes).to include(*application_scopes.split(/ /)) + end + end + + context 'the application has all scopes' do + let(:application_scopes) { 'api read_api read_user' } + + include_examples 'downgrades scopes' + end + + context 'the application has api and read_user scopes' do + let(:application_scopes) { 'api read_user' } + + include_examples 'downgrades scopes' + end + + context 'the application has read_api and read_user scopes' do + let(:application_scopes) { 'read_api read_user' } + + include_examples 'downgrades scopes' + end + + context 'the application has only api scopes' do + let(:application_scopes) { 'api' } + + include_examples 'downgrades scopes' + include_examples 'adds read_user scope' + end + + context 'the application has only read_api scopes' do + let(:application_scopes) { 'read_api' } + + include_examples 'downgrades scopes' + include_examples 'adds read_user scope' + end + + context 'the application has scopes we do not handle' do + let(:application_scopes) { Gitlab::Auth::PROFILE_SCOPE.to_s } + + before do + params[:scope] = application_scopes + end + + it 'does not modify the scopes' do + subject + + pre_auth = controller.send(:pre_auth) + + expect(pre_auth.scopes).to contain_exactly(application_scopes) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/new') + end + end + end end end end diff --git a/spec/features/merge_request/user_comments_on_diff_spec.rb b/spec/features/merge_request/user_comments_on_diff_spec.rb index f9b554c5ed2..8f7ad9bd90e 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -158,7 +158,7 @@ RSpec.describe 'User comments on a diff', :js do it 'allows comments on previously hidden lines at the bottom of a file' do # Click +28, expand down, select 37 add and verify comment page.within('[data-path="files/ruby/popen.rb"]') do - all('.js-unfold-down')[1].click + all('.js-unfold-down:not([disabled])')[1].click end click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="30"]').find(:xpath, '../..'), 'left') add_comment('+28', '37') diff --git a/spec/features/merge_requests/user_mass_updates_spec.rb b/spec/features/merge_requests/user_mass_updates_spec.rb index 46c12784ea8..f781ba0827c 100644 --- a/spec/features/merge_requests/user_mass_updates_spec.rb +++ b/spec/features/merge_requests/user_mass_updates_spec.rb @@ -8,6 +8,8 @@ RSpec.describe 'Merge requests > User mass updates', :js do let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } before do + stub_feature_flags(mr_attention_requests: false) + project.add_maintainer(user) sign_in(user) end @@ -59,6 +61,18 @@ RSpec.describe 'Merge requests > User mass updates', :js do expect(find('.merge-request')).to have_link "Assigned to #{user.name}" end + + describe 'with attention requests feature flag on' do + before do + stub_feature_flags(mr_attention_requests: true) + end + + it 'updates merge request with assignee' do + change_assignee(user.name) + + expect(find('.issuable-meta a.author-link')[:title]).to eq "Attention requested from assignee #{user.name}, go to their profile." + end + end end describe 'remove assignee' do diff --git a/spec/frontend/cycle_analytics/metric_popover_spec.js b/spec/frontend/analytics/shared/components/metric_popover_spec.js index 5a622fcacd5..b799c911488 100644 --- a/spec/frontend/cycle_analytics/metric_popover_spec.js +++ b/spec/frontend/analytics/shared/components/metric_popover_spec.js @@ -1,6 +1,6 @@ import { GlLink, GlIcon } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import MetricPopover from '~/cycle_analytics/components/metric_popover.vue'; +import MetricPopover from '~/analytics/shared/components/metric_popover.vue'; const MOCK_METRIC = { key: 'deployment-frequency', diff --git a/spec/frontend/cycle_analytics/metric_tile_spec.js b/spec/frontend/analytics/shared/components/metric_tile_spec.js index bebf3b8a65f..980dfad9eb0 100644 --- a/spec/frontend/cycle_analytics/metric_tile_spec.js +++ b/spec/frontend/analytics/shared/components/metric_tile_spec.js @@ -1,7 +1,7 @@ import { GlSingleStat } from '@gitlab/ui/dist/charts'; import { shallowMount } from '@vue/test-utils'; -import MetricTile from '~/cycle_analytics/components/metric_tile.vue'; -import MetricPopover from '~/cycle_analytics/components/metric_popover.vue'; +import MetricTile from '~/analytics/shared/components/metric_tile.vue'; +import MetricPopover from '~/analytics/shared/components/metric_popover.vue'; import { redirectTo } from '~/lib/utils/url_utility'; jest.mock('~/lib/utils/url_utility'); diff --git a/spec/frontend/analytics/shared/utils_spec.js b/spec/frontend/analytics/shared/utils_spec.js index 0513ccb2890..b48e2d971b5 100644 --- a/spec/frontend/analytics/shared/utils_spec.js +++ b/spec/frontend/analytics/shared/utils_spec.js @@ -1,9 +1,12 @@ +import metricsData from 'test_fixtures/projects/analytics/value_stream_analytics/summary.json'; import { filterBySearchTerm, extractFilterQueryParameters, extractPaginationQueryParameters, getDataZoomOption, + prepareTimeMetricsData, } from '~/analytics/shared/utils'; +import { slugify } from '~/lib/utils/text_utility'; import { objectToQuery } from '~/lib/utils/url_utility'; describe('filterBySearchTerm', () => { @@ -176,3 +179,36 @@ describe('getDataZoomOption', () => { }); }); }); + +describe('prepareTimeMetricsData', () => { + let prepared; + const [first, second] = metricsData; + delete second.identifier; // testing the case when identifier is missing + + const firstIdentifier = first.identifier; + const secondIdentifier = slugify(second.title); + + beforeEach(() => { + prepared = prepareTimeMetricsData([first, second], { + [firstIdentifier]: { description: 'Is a value that is good' }, + }); + }); + + it('will add a `identifier` based on the title', () => { + expect(prepared).toMatchObject([ + { identifier: firstIdentifier }, + { identifier: secondIdentifier }, + ]); + }); + + it('will add a `label` key', () => { + expect(prepared).toMatchObject([{ label: 'New Issues' }, { label: 'Commits' }]); + }); + + it('will add a popover description using the key if it is provided', () => { + expect(prepared).toMatchObject([ + { description: 'Is a value that is good' }, + { description: '' }, + ]); + }); +}); diff --git a/spec/frontend/cycle_analytics/base_spec.js b/spec/frontend/cycle_analytics/base_spec.js index 9a9415cc12a..7b1ef71da63 100644 --- a/spec/frontend/cycle_analytics/base_spec.js +++ b/spec/frontend/cycle_analytics/base_spec.js @@ -3,11 +3,11 @@ import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; import Vuex from 'vuex'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import ValueStreamMetrics from '~/analytics/shared/components/value_stream_metrics.vue'; import BaseComponent from '~/cycle_analytics/components/base.vue'; import PathNavigation from '~/cycle_analytics/components/path_navigation.vue'; import StageTable from '~/cycle_analytics/components/stage_table.vue'; import ValueStreamFilters from '~/cycle_analytics/components/value_stream_filters.vue'; -import ValueStreamMetrics from '~/cycle_analytics/components/value_stream_metrics.vue'; import { NOT_ENOUGH_DATA_ERROR } from '~/cycle_analytics/constants'; import initState from '~/cycle_analytics/store/state'; import { diff --git a/spec/frontend/cycle_analytics/utils_spec.js b/spec/frontend/cycle_analytics/utils_spec.js index f98144d92a5..51405a1ba4d 100644 --- a/spec/frontend/cycle_analytics/utils_spec.js +++ b/spec/frontend/cycle_analytics/utils_spec.js @@ -1,13 +1,10 @@ -import metricsData from 'test_fixtures/projects/analytics/value_stream_analytics/summary.json'; import { transformStagesForPathNavigation, medianTimeToParsedSeconds, formatMedianValues, filterStagesByHiddenStatus, - prepareTimeMetricsData, buildCycleAnalyticsInitialData, } from '~/cycle_analytics/utils'; -import { slugify } from '~/lib/utils/text_utility'; import { selectedStage, allowedStages, @@ -89,39 +86,6 @@ describe('Value stream analytics utils', () => { }); }); - describe('prepareTimeMetricsData', () => { - let prepared; - const [first, second] = metricsData; - delete second.identifier; // testing the case when identifier is missing - - const firstIdentifier = first.identifier; - const secondIdentifier = slugify(second.title); - - beforeEach(() => { - prepared = prepareTimeMetricsData([first, second], { - [firstIdentifier]: { description: 'Is a value that is good' }, - }); - }); - - it('will add a `identifier` based on the title', () => { - expect(prepared).toMatchObject([ - { identifier: firstIdentifier }, - { identifier: secondIdentifier }, - ]); - }); - - it('will add a `label` key', () => { - expect(prepared).toMatchObject([{ label: 'New Issues' }, { label: 'Commits' }]); - }); - - it('will add a popover description using the key if it is provided', () => { - expect(prepared).toMatchObject([ - { description: 'Is a value that is good' }, - { description: '' }, - ]); - }); - }); - describe('buildCycleAnalyticsInitialData', () => { let res = null; const projectId = '5'; diff --git a/spec/frontend/cycle_analytics/value_stream_metrics_spec.js b/spec/frontend/cycle_analytics/value_stream_metrics_spec.js index a2d37699387..7a539b262fc 100644 --- a/spec/frontend/cycle_analytics/value_stream_metrics_spec.js +++ b/spec/frontend/cycle_analytics/value_stream_metrics_spec.js @@ -3,11 +3,11 @@ import { shallowMount } from '@vue/test-utils'; import { nextTick } from 'vue'; import metricsData from 'test_fixtures/projects/analytics/value_stream_analytics/summary.json'; import waitForPromises from 'helpers/wait_for_promises'; +import ValueStreamMetrics from '~/analytics/shared/components/value_stream_metrics.vue'; import { METRIC_TYPE_SUMMARY } from '~/api/analytics_api'; -import ValueStreamMetrics from '~/cycle_analytics/components/value_stream_metrics.vue'; -import { METRICS_POPOVER_CONTENT } from '~/cycle_analytics/constants'; -import { prepareTimeMetricsData } from '~/cycle_analytics/utils'; -import MetricTile from '~/cycle_analytics/components/metric_tile.vue'; +import { METRICS_POPOVER_CONTENT } from '~/analytics/shared/constants'; +import { prepareTimeMetricsData } from '~/analytics/shared/utils'; +import MetricTile from '~/analytics/shared/components/metric_tile.vue'; import createFlash from '~/flash'; import { group } from './mock_data'; diff --git a/spec/frontend/diffs/components/diff_expansion_cell_spec.js b/spec/frontend/diffs/components/diff_expansion_cell_spec.js index f53f10d955d..cd472920bb9 100644 --- a/spec/frontend/diffs/components/diff_expansion_cell_spec.js +++ b/spec/frontend/diffs/components/diff_expansion_cell_spec.js @@ -91,7 +91,9 @@ describe('DiffExpansionCell', () => { }); expect(findExpandUp(wrapper).exists()).toBe(true); - expect(findExpandDown(wrapper).exists()).toBe(false); + expect(findExpandDown(wrapper).exists()).toBe(true); + expect(findExpandUp(wrapper).attributes('disabled')).not.toBeDefined(); + expect(findExpandDown(wrapper).attributes('disabled')).toBeDefined(); expect(findExpandAll(wrapper)).not.toBe(null); }); }); @@ -112,8 +114,10 @@ describe('DiffExpansionCell', () => { isBottom: true, }); - expect(findExpandUp(wrapper).exists()).toBe(false); expect(findExpandDown(wrapper).exists()).toBe(true); + expect(findExpandUp(wrapper).exists()).toBe(true); + expect(findExpandDown(wrapper).attributes('disabled')).not.toBeDefined(); + expect(findExpandUp(wrapper).attributes('disabled')).toBeDefined(); expect(findExpandAll(wrapper)).not.toBe(null); }); }); diff --git a/spec/frontend/google_tag_manager/index_spec.js b/spec/frontend/google_tag_manager/index_spec.js index f9199f32f1e..9112b0e17e7 100644 --- a/spec/frontend/google_tag_manager/index_spec.js +++ b/spec/frontend/google_tag_manager/index_spec.js @@ -224,8 +224,8 @@ describe('~/google_tag_manager/index', () => { { ecommerce: null }, { event: 'EECCheckout', - currencyCode: 'USD', ecommerce: { + currencyCode: 'USD', checkout: { actionField: { step: 1 }, products: [ @@ -254,8 +254,8 @@ describe('~/google_tag_manager/index', () => { expect(spy).toHaveBeenCalledWith({ ecommerce: null }); expect(spy).toHaveBeenCalledWith({ event: 'EECCheckout', - currencyCode: 'USD', ecommerce: { + currencyCode: 'USD', checkout: { actionField: { step: 1 }, products: [ @@ -291,8 +291,8 @@ describe('~/google_tag_manager/index', () => { expect(spy).toHaveBeenCalledWith({ ecommerce: null }); expect(spy).toHaveBeenCalledWith({ event: 'EECCheckout', - currencyCode: 'USD', ecommerce: { + currencyCode: 'USD', checkout: { actionField: { step: 1 }, products: [ @@ -347,8 +347,8 @@ describe('~/google_tag_manager/index', () => { { ecommerce: null }, { event: 'EECtransactionSuccess', - currencyCode: 'USD', ecommerce: { + currencyCode: 'USD', purchase: { actionField: { id: '123', diff --git a/spec/frontend/lib/utils/table_utility_spec.js b/spec/frontend/lib/utils/table_utility_spec.js index a67adac5efb..0ceccbe4c74 100644 --- a/spec/frontend/lib/utils/table_utility_spec.js +++ b/spec/frontend/lib/utils/table_utility_spec.js @@ -30,8 +30,10 @@ describe('table_utility', () => { ${'mergedAt'} | ${false} | ${'MERGED_AT_ASC'} ${'severity'} | ${true} | ${'SEVERITY_DESC'} ${'severity'} | ${false} | ${'SEVERITY_ASC'} + ${null} | ${null} | ${'SEVERITY'} + ${null} | ${null} | ${''} `( - 'returns $sortString when sortBy = "$sortBy" and sortDesc = "sortDesc"', + 'returns the expected sort object when the sort string is "$sortString"', ({ sortBy, sortDesc, sortString }) => { expect(tableUtils.sortStringToObject(sortString)).toStrictEqual({ sortBy, sortDesc }); }, diff --git a/spec/frontend/terraform/components/states_table_spec.js b/spec/frontend/terraform/components/states_table_spec.js index 9dbc8335851..fa9c8320b4f 100644 --- a/spec/frontend/terraform/components/states_table_spec.js +++ b/spec/frontend/terraform/components/states_table_spec.js @@ -1,4 +1,4 @@ -import { GlIcon, GlLoadingIcon, GlTooltip } from '@gitlab/ui'; +import { GlBadge, GlLoadingIcon, GlTooltip } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import { nextTick } from 'vue'; import { useFakeDate } from 'helpers/fake_date'; @@ -139,7 +139,7 @@ describe('StatesTable', () => { const toolTip = state.find(GlTooltip); expect(state.text()).toContain(name); - expect(state.find(GlIcon).exists()).toBe(locked); + expect(state.find(GlBadge).exists()).toBe(locked); expect(state.find(GlLoadingIcon).exists()).toBe(loading); expect(toolTip.exists()).toBe(locked); diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index fc93910344c..706344831b8 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -10,29 +10,29 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do describe 'constants' do it 'API_SCOPES contains all scopes for API access' do - expect(subject::API_SCOPES).to eq %i[api read_user read_api] + expect(subject::API_SCOPES).to match_array %i[api read_user read_api] end it 'ADMIN_SCOPES contains all scopes for ADMIN access' do - expect(subject::ADMIN_SCOPES).to eq %i[sudo] + expect(subject::ADMIN_SCOPES).to match_array %i[sudo] end it 'REPOSITORY_SCOPES contains all scopes for REPOSITORY access' do - expect(subject::REPOSITORY_SCOPES).to eq %i[read_repository write_repository] + expect(subject::REPOSITORY_SCOPES).to match_array %i[read_repository write_repository] end it 'OPENID_SCOPES contains all scopes for OpenID Connect' do - expect(subject::OPENID_SCOPES).to eq [:openid] + expect(subject::OPENID_SCOPES).to match_array [:openid] end it 'DEFAULT_SCOPES contains all default scopes' do - expect(subject::DEFAULT_SCOPES).to eq [:api] + expect(subject::DEFAULT_SCOPES).to match_array [:api] end it 'optional_scopes contains all non-default scopes' do stub_container_registry_config(enabled: true) - expect(subject.optional_scopes).to eq %i[read_user read_api read_repository write_repository read_registry write_registry sudo openid profile email] + expect(subject.optional_scopes).to match_array %i[read_user read_api read_repository write_repository read_registry write_registry sudo openid profile email] end end @@ -40,21 +40,21 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'contains all non-default scopes' do stub_container_registry_config(enabled: true) - expect(subject.all_available_scopes).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry sudo] + expect(subject.all_available_scopes).to match_array %i[api read_user read_api read_repository write_repository read_registry write_registry sudo] end it 'contains for non-admin user all non-default scopes without ADMIN access' do stub_container_registry_config(enabled: true) user = create(:user, admin: false) - expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry] + expect(subject.available_scopes_for(user)).to match_array %i[api read_user read_api read_repository write_repository read_registry write_registry] end it 'contains for admin user all non-default scopes with ADMIN access' do stub_container_registry_config(enabled: true) user = create(:user, admin: true) - expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry sudo] + expect(subject.available_scopes_for(user)).to match_array %i[api read_user read_api read_repository write_repository read_registry write_registry sudo] end context 'registry_scopes' do diff --git a/spec/lib/gitlab/omniauth_initializer_spec.rb b/spec/lib/gitlab/omniauth_initializer_spec.rb index 577d15b8495..42ae5844b95 100644 --- a/spec/lib/gitlab/omniauth_initializer_spec.rb +++ b/spec/lib/gitlab/omniauth_initializer_spec.rb @@ -101,5 +101,19 @@ RSpec.describe Gitlab::OmniauthInitializer do subject.execute([google_config]) end + + it 'configures defaults for gitlab' do + conf = { + 'name' => 'gitlab', + "args" => {} + } + + expect(devise_config).to receive(:omniauth).with( + :gitlab, + authorize_params: { gl_auth_type: 'login' } + ) + + subject.execute([conf]) + end end end diff --git a/spec/lib/gitlab/usage_data_counters/jetbrains_plugin_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/jetbrains_plugin_activity_unique_counter_spec.rb index 64fa8d26d81..4169546edad 100644 --- a/spec/lib/gitlab/usage_data_counters/jetbrains_plugin_activity_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/jetbrains_plugin_activity_unique_counter_spec.rb @@ -2,62 +2,14 @@ require 'spec_helper' -RSpec.shared_examples 'a tracked jetbrains unique action' do |event| - before do - stub_application_setting(usage_ping_enabled: true) - end - - def count_unique(date_from:, date_to:) - Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: action, start_date: date_from, end_date: date_to) - end - - it 'tracks when the user agent is from jetbrains' do - aggregate_failures do - user_agent = { user_agent: 'gitlab-jetbrains-plugin/0.0.1 intellij-idea/2021.2.4 java/11.0.13 mac-os-x/aarch64/12.1' } - - expect(track_action(user: user1, **user_agent)).to be_truthy - expect(track_action(user: user1, **user_agent)).to be_truthy - expect(track_action(user: user2, **user_agent)).to be_truthy - - expect(count_unique(date_from: time - 1.week, date_to: time + 1.week)).to eq(2) - end - end - - it 'does not track when the user agent is not from jetbrains' do - aggregate_failures do - user_agent = { user_agent: 'normal_user_agent' } - - expect(track_action(user: user1, **user_agent)).to be_falsey - expect(track_action(user: user1, **user_agent)).to be_falsey - expect(track_action(user: user2, **user_agent)).to be_falsey - - expect(count_unique(date_from: time - 1.week, date_to: time + 1.week)).to eq(0) - end - end - - it 'does not track if user agent is not present' do - expect(track_action(user: nil, user_agent: nil)).to be_nil - end - - it 'does not track if user is not present' do - user_agent = { user_agent: 'gitlab-jetbrains-plugin/0.0.1 intellij-idea/2021.2.4 java/11.0.13 mac-os-x/aarch64/12.1' } - - expect(track_action(user: nil, **user_agent)).to be_nil - end -end - -RSpec.describe Gitlab::UsageDataCounters::JetBrainsPluginActivityUniqueCounter, :clean_gitlab_redis_shared_state do +RSpec.describe Gitlab::UsageDataCounters::JetBrainsPluginActivityUniqueCounter, :clean_gitlab_redis_shared_state do # rubocop:disable RSpec/FilePath let(:user1) { build(:user, id: 1) } let(:user2) { build(:user, id: 2) } let(:time) { Time.current } + let(:action) { described_class::JETBRAINS_API_REQUEST_ACTION } + let(:user_agent) { { user_agent: 'gitlab-jetbrains-plugin/0.0.1 intellij-idea/2021.2.4 java/11.0.13 mac-os-x/aarch64/12.1' } } context 'when tracking a jetbrains api request' do - it_behaves_like 'a tracked jetbrains unique action' do - let(:action) { described_class::JETBRAINS_API_REQUEST_ACTION } - - def track_action(params) - described_class.track_api_request_when_trackable(**params) - end - end + it_behaves_like 'a request from an extension' end end diff --git a/spec/lib/gitlab/usage_data_counters/vscode_extension_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/vscode_extension_activity_unique_counter_spec.rb new file mode 100644 index 00000000000..640dadd8c0b --- /dev/null +++ b/spec/lib/gitlab/usage_data_counters/vscode_extension_activity_unique_counter_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::UsageDataCounters::VSCodeExtensionActivityUniqueCounter, :clean_gitlab_redis_shared_state do # rubocop:disable RSpec/FilePath + let(:user1) { build(:user, id: 1) } + let(:user2) { build(:user, id: 2) } + let(:time) { Time.current } + let(:action) { described_class::VS_CODE_API_REQUEST_ACTION } + let(:user_agent) { { user_agent: 'vs-code-gitlab-workflow/3.11.1 VSCode/1.52.1 Node.js/12.14.1 (darwin; x64)' } } + + context 'when tracking a vs code api request' do + it_behaves_like 'a request from an extension' + end +end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 8ee752da44e..70a310ba0d5 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -275,7 +275,7 @@ RSpec.describe 'OpenID Connect requests' do expect(response).to have_gitlab_http_status(:ok) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to eq(%w[api read_user read_api read_repository write_repository sudo openid profile email]) + expect(json_response['scopes_supported']).to match_array %w[api read_user read_api read_repository write_repository sudo openid profile email] end context 'with a cross-origin request' do @@ -285,7 +285,7 @@ RSpec.describe 'OpenID Connect requests' do expect(response).to have_gitlab_http_status(:ok) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to eq(%w[api read_user read_api read_repository write_repository sudo openid profile email]) + expect(json_response['scopes_supported']).to match_array %w[api read_user read_api read_repository write_repository sudo openid profile email] end it_behaves_like 'cross-origin GET request' diff --git a/spec/services/ci/process_sync_events_service_spec.rb b/spec/services/ci/process_sync_events_service_spec.rb index 3f4b2a0f7f2..6b9717fe57d 100644 --- a/spec/services/ci/process_sync_events_service_spec.rb +++ b/spec/services/ci/process_sync_events_service_spec.rb @@ -25,6 +25,8 @@ RSpec.describe Ci::ProcessSyncEventsService do project2.update!(group: parent_group_2) end + it { is_expected.to eq(service_results(2, 2, 2)) } + it 'consumes events' do expect { execute }.to change(Projects::SyncEvent, :count).from(2).to(0) @@ -36,20 +38,32 @@ RSpec.describe Ci::ProcessSyncEventsService do ) end - it 'enqueues Projects::ProcessSyncEventsWorker if any left' do - stub_const("#{described_class}::BATCH_SIZE", 1) + context 'when any event left after processing' do + before do + stub_const("#{described_class}::BATCH_SIZE", 1) + end - expect(Projects::ProcessSyncEventsWorker).to receive(:perform_async) + it { is_expected.to eq(service_results(2, 1, 1)) } - execute + it 'enqueues Projects::ProcessSyncEventsWorker' do + expect(Projects::ProcessSyncEventsWorker).to receive(:perform_async) + + execute + end end - it 'does not enqueue Projects::ProcessSyncEventsWorker if no left' do - stub_const("#{described_class}::BATCH_SIZE", 2) + context 'when no event left after processing' do + before do + stub_const("#{described_class}::BATCH_SIZE", 2) + end - expect(Projects::ProcessSyncEventsWorker).not_to receive(:perform_async) + it { is_expected.to eq(service_results(2, 2, 2)) } - execute + it 'does not enqueue Projects::ProcessSyncEventsWorker' do + expect(Projects::ProcessSyncEventsWorker).not_to receive(:perform_async) + + execute + end end context 'when there is no event' do @@ -57,27 +71,45 @@ RSpec.describe Ci::ProcessSyncEventsService do Projects::SyncEvent.delete_all end + it { is_expected.to eq(service_results(0, 0, nil)) } + it 'does nothing' do expect { execute }.not_to change(Projects::SyncEvent, :count) end end - it 'does not delete non-executed events' do - new_project = create(:project) - sync_event_class.delete_all + context 'when there is non-executed events' do + before do + new_project = create(:project) + sync_event_class.delete_all - project1.update!(group: parent_group_2) - new_project.update!(group: parent_group_1) - project2.update!(group: parent_group_1) + project1.update!(group: parent_group_2) + new_project.update!(group: parent_group_1) + project2.update!(group: parent_group_1) - new_project_sync_event = new_project.sync_events.last + @new_project_sync_event = new_project.sync_events.last - allow(sync_event_class).to receive(:preload_synced_relation).and_return( - sync_event_class.where.not(id: new_project_sync_event) - ) + allow(sync_event_class).to receive(:preload_synced_relation).and_return( + sync_event_class.where.not(id: @new_project_sync_event) + ) + end + + it { is_expected.to eq(service_results(3, 2, 2)) } + + it 'does not delete non-executed events' do + expect { execute }.to change(Projects::SyncEvent, :count).from(3).to(1) + expect(@new_project_sync_event.reload).to be_persisted + end + end + + private - expect { execute }.to change(Projects::SyncEvent, :count).from(3).to(1) - expect(new_project_sync_event.reload).to be_persisted + def service_results(total, consumable, processed) + { + estimated_total_events: total, + consumable_events: consumable, + processed_events: processed + }.compact end end diff --git a/spec/lib/gitlab/usage_data_counters/vscode_extenion_activity_unique_counter_spec.rb b/spec/support/shared_examples/lib/gitlab/usage_data_counters/code_review_extension_request_examples.rb index 7593d51fe76..6221366ab51 100644 --- a/spec/lib/gitlab/usage_data_counters/vscode_extenion_activity_unique_counter_spec.rb +++ b/spec/support/shared_examples/lib/gitlab/usage_data_counters/code_review_extension_request_examples.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.shared_examples 'a tracked vs code unique action' do |event| +RSpec.shared_examples 'a request from an extension' do |event| before do stub_application_setting(usage_ping_enabled: true) end @@ -11,10 +11,12 @@ RSpec.shared_examples 'a tracked vs code unique action' do |event| Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: action, start_date: date_from, end_date: date_to) end - it 'tracks when the user agent is from vs code' do - aggregate_failures do - user_agent = { user_agent: 'vs-code-gitlab-workflow/3.11.1 VSCode/1.52.1 Node.js/12.14.1 (darwin; x64)' } + def track_action(params) + described_class.track_api_request_when_trackable(**params) + end + it 'tracks when the user agent is matching' do + aggregate_failures do expect(track_action(user: user1, **user_agent)).to be_truthy expect(track_action(user: user1, **user_agent)).to be_truthy expect(track_action(user: user2, **user_agent)).to be_truthy @@ -23,7 +25,7 @@ RSpec.shared_examples 'a tracked vs code unique action' do |event| end end - it 'does not track when the user agent is not from vs code' do + it 'does not track when the user agent is not matching' do aggregate_failures do user_agent = { user_agent: 'normal_user_agent' } @@ -40,24 +42,6 @@ RSpec.shared_examples 'a tracked vs code unique action' do |event| end it 'does not track if user is not present' do - user_agent = { user_agent: 'vs-code-gitlab-workflow/3.11.1 VSCode/1.52.1 Node.js/12.14.1 (darwin; x64)' } - expect(track_action(user: nil, **user_agent)).to be_nil end end - -RSpec.describe Gitlab::UsageDataCounters::VSCodeExtensionActivityUniqueCounter, :clean_gitlab_redis_shared_state do - let(:user1) { build(:user, id: 1) } - let(:user2) { build(:user, id: 2) } - let(:time) { Time.current } - - context 'when tracking a vs code api request' do - it_behaves_like 'a tracked vs code unique action' do - let(:action) { described_class::VS_CODE_API_REQUEST_ACTION } - - def track_action(params) - described_class.track_api_request_when_trackable(**params) - end - end - end -end diff --git a/spec/workers/namespaces/process_sync_events_worker_spec.rb b/spec/workers/namespaces/process_sync_events_worker_spec.rb index 59be1fffdb4..c15a74a2934 100644 --- a/spec/workers/namespaces/process_sync_events_worker_spec.rb +++ b/spec/workers/namespaces/process_sync_events_worker_spec.rb @@ -7,10 +7,12 @@ RSpec.describe Namespaces::ProcessSyncEventsWorker do let!(:group2) { create(:group) } let!(:group3) { create(:group) } + subject(:worker) { described_class.new } + include_examples 'an idempotent worker' describe '#perform' do - subject(:perform) { described_class.new.perform } + subject(:perform) { worker.perform } before do group2.update!(parent: group1) @@ -28,5 +30,13 @@ RSpec.describe Namespaces::ProcessSyncEventsWorker do an_object_having_attributes(namespace_id: group3.id, traversal_ids: [group1.id, group2.id, group3.id]) ) end + + it 'logs the service result', :aggregate_failures do + expect(worker).to receive(:log_extra_metadata_on_done).with(:estimated_total_events, 5) + expect(worker).to receive(:log_extra_metadata_on_done).with(:consumable_events, 5) + expect(worker).to receive(:log_extra_metadata_on_done).with(:processed_events, 5) + + perform + end end end diff --git a/spec/workers/projects/process_sync_events_worker_spec.rb b/spec/workers/projects/process_sync_events_worker_spec.rb index 600fbbc6b20..963e0ad1028 100644 --- a/spec/workers/projects/process_sync_events_worker_spec.rb +++ b/spec/workers/projects/process_sync_events_worker_spec.rb @@ -6,10 +6,12 @@ RSpec.describe Projects::ProcessSyncEventsWorker do let!(:group) { create(:group) } let!(:project) { create(:project) } + subject(:worker) { described_class.new } + include_examples 'an idempotent worker' describe '#perform' do - subject(:perform) { described_class.new.perform } + subject(:perform) { worker.perform } before do project.update!(namespace: group) @@ -24,5 +26,13 @@ RSpec.describe Projects::ProcessSyncEventsWorker do an_object_having_attributes(namespace_id: group.id) ) end + + it 'logs the service result', :aggregate_failures do + expect(worker).to receive(:log_extra_metadata_on_done).with(:estimated_total_events, 2) + expect(worker).to receive(:log_extra_metadata_on_done).with(:consumable_events, 2) + expect(worker).to receive(:log_extra_metadata_on_done).with(:processed_events, 2) + + perform + end end end |