diff options
68 files changed, 1621 insertions, 289 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 288905fbd39..5797c04c518 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1be74fe6af19847eec28665da39ef03865329acb +0cea2923073bcd867dd8e718a0a7b4f7de5b6094 diff --git a/app/assets/javascripts/graphql_shared/fragments/issuable_timelogs.fragment.graphql b/app/assets/javascripts/graphql_shared/fragments/issuable_timelogs.fragment.graphql new file mode 100644 index 00000000000..6ed3be84cd8 --- /dev/null +++ b/app/assets/javascripts/graphql_shared/fragments/issuable_timelogs.fragment.graphql @@ -0,0 +1,10 @@ +fragment TimelogFragment on Timelog { + timeSpent + user { + name + } + spentAt + note { + body + } +} diff --git a/app/assets/javascripts/jobs/components/table/cells/job_cell.vue b/app/assets/javascripts/jobs/components/table/cells/job_cell.vue index 1797992641d..46108572a88 100644 --- a/app/assets/javascripts/jobs/components/table/cells/job_cell.vue +++ b/app/assets/javascripts/jobs/components/table/cells/job_cell.vue @@ -1,11 +1,18 @@ <script> -import { GlBadge, GlIcon, GlLink } from '@gitlab/ui'; +import { GlBadge, GlIcon, GlLink, GlTooltipDirective } from '@gitlab/ui'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; +import { s__ } from '~/locale'; import { SUCCESS_STATUS } from '../../../constants'; export default { iconSize: 12, badgeSize: 'sm', + i18n: { + stuckText: s__('Jobs|Job is stuck. Check runners.'), + }, + directives: { + GlTooltip: GlTooltipDirective, + }, components: { GlBadge, GlIcon, @@ -55,6 +62,9 @@ export default { canReadJob() { return this.job?.userPermissions?.readBuild; }, + jobStuck() { + return this.job?.stuck; + }, }, }; </script> @@ -73,6 +83,14 @@ export default { <span v-else data-testid="job-id-limited-access">{{ jobId }}</span> + <gl-icon + v-if="jobStuck" + v-gl-tooltip="$options.i18n.stuckText" + name="warning" + :size="$options.iconSize" + data-testid="stuck-icon" + /> + <div class="gl-display-flex gl-align-items-center"> <div v-if="jobRef" class="gl-max-w-15 gl-text-truncate"> <gl-icon diff --git a/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql b/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql index 2e49d07df5f..c2104754bad 100644 --- a/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql +++ b/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql @@ -59,6 +59,7 @@ query getJobs($fullPath: ID!, $statuses: [CiJobStatus!]) { playable cancelable active + stuck userPermissions { readBuild } diff --git a/app/assets/javascripts/sidebar/components/time_tracking/report.vue b/app/assets/javascripts/sidebar/components/time_tracking/report.vue new file mode 100644 index 00000000000..6764c5df06a --- /dev/null +++ b/app/assets/javascripts/sidebar/components/time_tracking/report.vue @@ -0,0 +1,102 @@ +<script> +import { GlLoadingIcon, GlTable } from '@gitlab/ui'; +import createFlash from '~/flash'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { formatDate, parseSeconds, stringifyTime } from '~/lib/utils/datetime_utility'; +import { __ } from '~/locale'; +import { timelogQueries } from '~/sidebar/constants'; + +const TIME_DATE_FORMAT = 'mmmm d, yyyy, HH:MM ("UTC:" o)'; + +export default { + components: { + GlLoadingIcon, + GlTable, + }, + inject: ['issuableId', 'issuableType'], + data() { + return { report: [], isLoading: true }; + }, + apollo: { + report: { + query() { + return timelogQueries[this.issuableType].query; + }, + variables() { + return { + id: convertToGraphQLId(this.getGraphQLEntityType(), this.issuableId), + }; + }, + update(data) { + this.isLoading = false; + return this.extractTimelogs(data); + }, + error() { + createFlash({ message: __('Something went wrong. Please try again.') }); + }, + }, + }, + methods: { + isIssue() { + return this.issuableType === 'issue'; + }, + getGraphQLEntityType() { + // eslint-disable-next-line @gitlab/require-i18n-strings + return this.isIssue() ? 'Issue' : 'MergeRequest'; + }, + extractTimelogs(data) { + const timelogs = data?.issuable?.timelogs?.nodes || []; + return timelogs.slice().sort((a, b) => new Date(a.spentAt) - new Date(b.spentAt)); + }, + formatDate(date) { + return formatDate(date, TIME_DATE_FORMAT); + }, + getNote(note) { + return note?.body; + }, + getTotalTimeSpent() { + const seconds = this.report.reduce((acc, item) => acc + item.timeSpent, 0); + return this.formatTimeSpent(seconds); + }, + formatTimeSpent(seconds) { + const negative = seconds < 0; + return (negative ? '- ' : '') + stringifyTime(parseSeconds(seconds)); + }, + }, + fields: [ + { key: 'spentAt', label: __('Spent At'), sortable: true }, + { key: 'user', label: __('User'), sortable: true }, + { key: 'timeSpent', label: __('Time Spent'), sortable: true }, + { key: 'note', label: __('Note'), sortable: true }, + ], +}; +</script> + +<template> + <div> + <div v-if="isLoading"><gl-loading-icon size="md" /></div> + <gl-table v-else :items="report" :fields="$options.fields" foot-clone> + <template #cell(spentAt)="{ item: { spentAt } }"> + <div>{{ formatDate(spentAt) }}</div> + </template> + <template #foot(spentAt)> </template> + + <template #cell(user)="{ item: { user } }"> + <div>{{ user.name }}</div> + </template> + <template #foot(user)> </template> + + <template #cell(timeSpent)="{ item: { timeSpent } }"> + <div>{{ formatTimeSpent(timeSpent) }}</div> + </template> + <template #foot(timeSpent)> + <div>{{ getTotalTimeSpent() }}</div> + </template> + + <template #cell(note)="{ item: { note } }"> + <div>{{ getNote(note) }}</div> + </template> + <template #foot(note)> </template> + </gl-table> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue index 4c095006dd7..ac3d278d840 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue @@ -1,10 +1,11 @@ <script> -import { GlIcon } from '@gitlab/ui'; +import { GlIcon, GlLink, GlModal, GlModalDirective } from '@gitlab/ui'; import { s__, __ } from '~/locale'; import eventHub from '../../event_hub'; import TimeTrackingCollapsedState from './collapsed_state.vue'; import TimeTrackingComparisonPane from './comparison_pane.vue'; import TimeTrackingHelpState from './help_state.vue'; +import TimeTrackingReport from './report.vue'; import TimeTrackingSpentOnlyPane from './spent_only_pane.vue'; export default { @@ -15,10 +16,16 @@ export default { }, components: { GlIcon, + GlLink, + GlModal, TimeTrackingCollapsedState, TimeTrackingSpentOnlyPane, TimeTrackingComparisonPane, TimeTrackingHelpState, + TimeTrackingReport, + }, + directives: { + GlModal: GlModalDirective, }, props: { timeEstimate: { @@ -160,6 +167,21 @@ export default { :time-estimate-human-readable="humanTimeEstimate" :limit-to-hours="limitToHours" /> + <gl-link + v-if="hasTimeSpent" + v-gl-modal="'time-tracking-report'" + data-testid="reportLink" + href="#" + class="btn-link" + >{{ __('Time tracking report') }}</gl-link + > + <gl-modal + modal-id="time-tracking-report" + :title="__('Time tracking report')" + :hide-footer="true" + > + <time-tracking-report /> + </gl-modal> <transition name="help-state-toggle"> <time-tracking-help-state v-if="showHelpState" /> </transition> diff --git a/app/assets/javascripts/sidebar/constants.js b/app/assets/javascripts/sidebar/constants.js index 3c747d19dc6..a4e6d8854d1 100644 --- a/app/assets/javascripts/sidebar/constants.js +++ b/app/assets/javascripts/sidebar/constants.js @@ -21,8 +21,10 @@ import updateIssueSubscriptionMutation from '~/sidebar/queries/update_issue_subs import updateMergeRequestSubscriptionMutation from '~/sidebar/queries/update_merge_request_subscription.mutation.graphql'; import getIssueAssignees from '~/vue_shared/components/sidebar/queries/get_issue_assignees.query.graphql'; import issueParticipantsQuery from '~/vue_shared/components/sidebar/queries/get_issue_participants.query.graphql'; +import getIssueTimelogsQuery from '~/vue_shared/components/sidebar/queries/get_issue_timelogs.query.graphql'; import getMergeRequestAssignees from '~/vue_shared/components/sidebar/queries/get_mr_assignees.query.graphql'; import getMergeRequestParticipants from '~/vue_shared/components/sidebar/queries/get_mr_participants.query.graphql'; +import getMrTimelogsQuery from '~/vue_shared/components/sidebar/queries/get_mr_timelogs.query.graphql'; import updateIssueAssigneesMutation from '~/vue_shared/components/sidebar/queries/update_issue_assignees.mutation.graphql'; import updateMergeRequestAssigneesMutation from '~/vue_shared/components/sidebar/queries/update_mr_assignees.mutation.graphql'; @@ -122,3 +124,12 @@ export const startDateQueries = { mutation: updateEpicStartDateMutation, }, }; + +export const timelogQueries = { + [IssuableType.Issue]: { + query: getIssueTimelogsQuery, + }, + [IssuableType.MergeRequest]: { + query: getMrTimelogsQuery, + }, +}; diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index d3bfa7c4b58..3f24fdc75dc 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -367,16 +367,16 @@ function mountSubscriptionsComponent() { function mountTimeTrackingComponent() { const el = document.getElementById('issuable-time-tracker'); + const { id, issuableType } = getSidebarOptions(); if (!el) return; // eslint-disable-next-line no-new new Vue({ el, - components: { - SidebarTimeTracking, - }, - render: (createElement) => createElement('sidebar-time-tracking', {}), + apolloProvider, + provide: { issuableId: id, issuableType }, + render: (createElement) => createElement(SidebarTimeTracking, {}), }); } diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js index bc8a8e425dd..3b2210b9ef2 100644 --- a/app/assets/javascripts/task_list.js +++ b/app/assets/javascripts/task_list.js @@ -40,20 +40,37 @@ export default class TaskList { taskListField.value = taskListField.dataset.value; }); - $(this.taskListContainerSelector).taskList('enable'); - $(document).on('tasklist:changed', this.taskListContainerSelector, this.updateHandler); + this.enable(); } getTaskListTarget(e) { return e && e.currentTarget ? $(e.currentTarget) : $(this.taskListContainerSelector); } + // Disable any task items that don't have a data-sourcepos attribute, on the + // assumption that if it doesn't then it wasn't generated from our markdown parser. + // This covers the case of markdown not being able to handle task lists inside + // markdown tables. It also includes hand coded HTML lists. + disableNonMarkdownTaskListItems(e) { + this.getTaskListTarget(e) + .find('.task-list-item') + .not('[data-sourcepos]') + .find('.task-list-item-checkbox') + .prop('disabled', true); + } + disableTaskListItems(e) { this.getTaskListTarget(e).taskList('disable'); } enableTaskListItems(e) { this.getTaskListTarget(e).taskList('enable'); + this.disableNonMarkdownTaskListItems(e); + } + + enable() { + this.enableTaskListItems(); + $(document).on('tasklist:changed', this.taskListContainerSelector, this.updateHandler); } disable() { diff --git a/app/assets/javascripts/vue_shared/components/sidebar/queries/get_issue_timelogs.query.graphql b/app/assets/javascripts/vue_shared/components/sidebar/queries/get_issue_timelogs.query.graphql new file mode 100644 index 00000000000..a2990d7171b --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/sidebar/queries/get_issue_timelogs.query.graphql @@ -0,0 +1,14 @@ +#import "~/graphql_shared/fragments/issuable_timelogs.fragment.graphql" + +query timeTrackingReport($id: IssueID!) { + issuable: issue(id: $id) { + __typename + id + title + timelogs { + nodes { + ...TimelogFragment + } + } + } +} diff --git a/app/assets/javascripts/vue_shared/components/sidebar/queries/get_mr_timelogs.query.graphql b/app/assets/javascripts/vue_shared/components/sidebar/queries/get_mr_timelogs.query.graphql new file mode 100644 index 00000000000..753f1b345e3 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/sidebar/queries/get_mr_timelogs.query.graphql @@ -0,0 +1,14 @@ +#import "~/graphql_shared/fragments/issuable_timelogs.fragment.graphql" + +query timeTrackingReport($id: MergeRequestID!) { + issuable: mergeRequest(id: $id) { + __typename + id + title + timelogs { + nodes { + ...TimelogFragment + } + } + } +} diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index ee9799ab71d..01739c7eb3e 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -672,11 +672,11 @@ $system-note-svg-size: 16px; align-items: center; margin-left: 10px; color: $gray-400; + margin-top: -4px; @include notes-media('max', map-get($grid-breakpoints, sm) - 1) { float: none; margin-left: 0; - transform: translateY(-4px); } } diff --git a/app/graphql/types/timelog_type.rb b/app/graphql/types/timelog_type.rb index 6314d6bca44..99a619f1b1d 100644 --- a/app/graphql/types/timelog_type.rb +++ b/app/graphql/types/timelog_type.rb @@ -43,5 +43,9 @@ module Types def issue Gitlab::Graphql::Loaders::BatchModelLoader.new(Issue, object.issue_id).find end + + def spent_at + object.spent_at || object.created_at + end end end diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index 6c4889de602..ecbc4972b60 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -89,6 +89,10 @@ module BoardsHelper @current_board_parent ||= @group || @project end + def current_board_namespace + @current_board_namespace = board.group_board? ? @group : @project.namespace + end + def can_update? can?(current_user, :admin_issue, board) end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 5750613e1f7..e2b25690323 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -33,7 +33,8 @@ class Deployment < ApplicationRecord scope :for_environment, -> (environment) { where(environment_id: environment) } scope :for_environment_name, -> (project, name) do - where(environment_id: Environment.select(:id).where(project: project, name: name)) + where('deployments.environment_id = (?)', + Environment.select(:id).where(project: project, name: name).limit(1)) end scope :for_status, -> (status) { where(status: status) } diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index 44d3c81aaaa..a28b97e63e5 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -4,6 +4,7 @@ class ProjectHook < WebHook include TriggerableHooks include Presentable include Limitable + extend ::Gitlab::Utils::Override self.limit_scope = :project @@ -33,6 +34,11 @@ class ProjectHook < WebHook def web_hooks_disable_failed? Feature.enabled?(:web_hooks_disable_failed, project) end + + override :rate_limit + def rate_limit + project.actual_limits.limit_for(:web_hook_calls) + end end ProjectHook.prepend_mod_with('ProjectHook') diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 44538134318..02b4feb4ccc 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -75,6 +75,11 @@ class WebHook < ApplicationRecord update!(recent_failures: 0, disabled_until: nil, backoff_count: 0) end + # Overridden in ProjectHook and GroupHook, other webhooks are not rate-limited. + def rate_limit + nil + end + private def web_hooks_disable_failed? diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index a3de5021076..654d9356739 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -91,7 +91,11 @@ class WebHookService end def async_execute - WebHookWorker.perform_async(hook.id, data, hook_name) + if rate_limited?(hook) + log_rate_limit(hook) + else + WebHookWorker.perform_async(hook.id, data, hook_name) + end end private @@ -170,4 +174,34 @@ class WebHookService response.body.encode('UTF-8', invalid: :replace, undef: :replace, replace: '') end + + def rate_limited?(hook) + return false unless Feature.enabled?(:web_hooks_rate_limit, default_enabled: :yaml) + return false if rate_limit.nil? + + Gitlab::ApplicationRateLimiter.throttled?( + :web_hook_calls, + scope: [hook], + threshold: rate_limit + ) + end + + def rate_limit + @rate_limit ||= hook.rate_limit + end + + def log_rate_limit(hook) + payload = { + message: 'Webhook rate limit exceeded', + hook_id: hook.id, + hook_type: hook.type, + hook_name: hook_name + } + + Gitlab::AuthLogger.error(payload) + + # Also log into application log for now, so we can use this information + # to determine suitable limits for gitlab.com + Gitlab::AppLogger.error(payload) + end end diff --git a/app/views/layouts/_loading_hints.html.haml b/app/views/layouts/_loading_hints.html.haml index 0ef50d1b122..cd1a236b6be 100644 --- a/app/views/layouts/_loading_hints.html.haml +++ b/app/views/layouts/_loading_hints.html.haml @@ -1,10 +1,11 @@ -- if ActionController::Base.asset_host - %link{ rel: 'dns-prefetch', href: ActionController::Base.asset_host } - %link{ rel: 'preconnect', href: ActionController::Base.asset_host, crossorigin: '' } -- if user_application_theme == 'gl-dark' - %link{ { rel: 'preload', href: stylesheet_url('application_dark'), as: 'style' }, ActionController::Base.asset_host ? { crossorigin: 'anonymous' } : {} } -- else - %link{ { rel: 'preload', href: stylesheet_url('application'), as: 'style' }, ActionController::Base.asset_host ? { crossorigin: 'anonymous' } : {} } -%link{ { rel: 'preload', href: stylesheet_url("highlight/themes/#{user_color_scheme}"), as: 'style' }, ActionController::Base.asset_host ? { crossorigin: 'anonymous' } : {} } -- if Gitlab::CurrentSettings.snowplow_enabled? && Gitlab::CurrentSettings.snowplow_collector_hostname - %link{ rel: 'preconnect', href: Gitlab::CurrentSettings.snowplow_collector_hostname, crossorigin: '' } += cache_if(Feature.enabled?(:cached_loading_hints, current_user), [ActionController::Base.asset_host, user_application_theme, user_color_scheme], expires_in: 1.minute) do + - if ActionController::Base.asset_host + %link{ rel: 'dns-prefetch', href: ActionController::Base.asset_host } + %link{ rel: 'preconnect', href: ActionController::Base.asset_host, crossorigin: '' } + - if user_application_theme == 'gl-dark' + %link{ { rel: 'preload', href: stylesheet_url('application_dark'), as: 'style' }, ActionController::Base.asset_host ? { crossorigin: 'anonymous' } : {} } + - else + %link{ { rel: 'preload', href: stylesheet_url('application'), as: 'style' }, ActionController::Base.asset_host ? { crossorigin: 'anonymous' } : {} } + %link{ { rel: 'preload', href: stylesheet_url("highlight/themes/#{user_color_scheme}"), as: 'style' }, ActionController::Base.asset_host ? { crossorigin: 'anonymous' } : {} } + - if Gitlab::CurrentSettings.snowplow_enabled? && Gitlab::CurrentSettings.snowplow_collector_hostname + %link{ rel: 'preconnect', href: Gitlab::CurrentSettings.snowplow_collector_hostname, crossorigin: '' } diff --git a/changelogs/unreleased/271409-time-tracking-reports.yml b/changelogs/unreleased/271409-time-tracking-reports.yml new file mode 100644 index 00000000000..84116ef6a32 --- /dev/null +++ b/changelogs/unreleased/271409-time-tracking-reports.yml @@ -0,0 +1,5 @@ +--- +title: Add isuable time tracking report +merge_request: 60161 +author: Lee Tickett @leetickett +type: added diff --git a/changelogs/unreleased/329207-rate-limit-webhooks.yml b/changelogs/unreleased/329207-rate-limit-webhooks.yml new file mode 100644 index 00000000000..134cdc67c35 --- /dev/null +++ b/changelogs/unreleased/329207-rate-limit-webhooks.yml @@ -0,0 +1,5 @@ +--- +title: Apply rate-limiting to webhook executions +merge_request: 61151 +author: +type: performance diff --git a/changelogs/unreleased/bw-table-task-list.yml b/changelogs/unreleased/bw-table-task-list.yml new file mode 100644 index 00000000000..a757b1bfa89 --- /dev/null +++ b/changelogs/unreleased/bw-table-task-list.yml @@ -0,0 +1,5 @@ +--- +title: Disable unsupported task items in Markdown tables +merge_request: 46060 +author: +type: fixed diff --git a/changelogs/unreleased/d-esterman-master-patch-46157.yml b/changelogs/unreleased/d-esterman-master-patch-46157.yml new file mode 100644 index 00000000000..e242e158e41 --- /dev/null +++ b/changelogs/unreleased/d-esterman-master-patch-46157.yml @@ -0,0 +1,5 @@ +--- +title: Bumped image in the CI-Template Jobs/Build.gitlab-ci.yml to 0.6.0 +merge_request: 59882 +author: Daniel Estermann (@d.esterman) +type: changed diff --git a/changelogs/unreleased/fix-deployment-finder-environment-in-query.yml b/changelogs/unreleased/fix-deployment-finder-environment-in-query.yml new file mode 100644 index 00000000000..cadf0251d26 --- /dev/null +++ b/changelogs/unreleased/fix-deployment-finder-environment-in-query.yml @@ -0,0 +1,5 @@ +--- +title: Fix environment filter of Deployments Finder +merge_request: 61564 +author: +type: performance diff --git a/changelogs/unreleased/fix-translatey-note-actions.yml b/changelogs/unreleased/fix-translatey-note-actions.yml new file mode 100644 index 00000000000..ed6210dc5a3 --- /dev/null +++ b/changelogs/unreleased/fix-translatey-note-actions.yml @@ -0,0 +1,5 @@ +--- +title: Fix position of note actions +merge_request: 61594 +author: +type: fixed diff --git a/config/feature_flags/development/cached_loading_hints.yml b/config/feature_flags/development/cached_loading_hints.yml new file mode 100644 index 00000000000..ba4eaece55a --- /dev/null +++ b/config/feature_flags/development/cached_loading_hints.yml @@ -0,0 +1,8 @@ +--- +name: cached_loading_hints +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61609 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/330802 +milestone: '13.12' +type: development +group: group::source code +default_enabled: false diff --git a/config/feature_flags/development/web_hooks_rate_limit.yml b/config/feature_flags/development/web_hooks_rate_limit.yml new file mode 100644 index 00000000000..193d51bb250 --- /dev/null +++ b/config/feature_flags/development/web_hooks_rate_limit.yml @@ -0,0 +1,8 @@ +--- +name: web_hooks_rate_limit +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61151 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/330133 +milestone: '13.12' +type: development +group: group::ecosystem +default_enabled: false diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index 7167ee5aad5..83c6f68869b 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -1,135 +1,3 @@ # frozen_string_literal: true -# rubocop:disable Style/SignalException -require 'yaml' - -SEE_DOC = "See the [changelog documentation](https://docs.gitlab.com/ee/development/changelog.html)." - -SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT -```suggestion -merge_request: %<mr_iid>s -``` - -#{SEE_DOC} -SUGGEST_COMMENT - -CATEGORIES = YAML - .load_file(File.expand_path('../../.gitlab/changelog_config.yml', __dir__)) - .fetch('categories') - .keys - .freeze - -def check_changelog_trailer(commit) - trailer = commit.message.match(/^Changelog:\s*(?<category>.+)$/) - - return :missing if trailer.nil? || trailer[:category].nil? - - category = trailer[:category] - - return :valid if CATEGORIES.include?(category) - - self.fail( - "Commit #{commit.sha} uses an invalid changelog category: #{category}" - ) - - :invalid -end - -def check_changelog_yaml(path) - raw_file = File.read(path) - yaml = YAML.safe_load(raw_file) - yaml_merge_request = yaml["merge_request"].to_s - - fail "`title` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil? - fail "`type` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil? - - return if helper.security_mr? - return if helper.mr_iid.empty? - - cherry_pick_against_stable_branch = helper.cherry_pick_mr? && helper.stable_branch? - - if yaml_merge_request.empty? - mr_line = raw_file.lines.find_index("merge_request:\n") - - if mr_line - markdown(format(SUGGEST_MR_COMMENT, mr_iid: helper.mr_iid), file: path, line: mr_line.succ) - else - message "Consider setting `merge_request` to #{helper.mr_iid} in #{helper.html_link(path)}. #{SEE_DOC}" - end - elsif yaml_merge_request != helper.mr_iid && !cherry_pick_against_stable_branch - fail "Merge request ID was not set to #{helper.mr_iid}! #{SEE_DOC}" - end -rescue Psych::Exception - # YAML could not be parsed, fail the build. - fail "#{helper.html_link(path)} isn't valid YAML! #{SEE_DOC}" -rescue StandardError => e - warn "There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}" -end - -def check_changelog_path(path) - ee_changes = project_helper.all_ee_changes.dup - ee_changes.delete(path) - - if ee_changes.any? && !changelog.ee_changelog? && !changelog.required? - warn "This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`." - end - - if ee_changes.empty? && changelog.ee_changelog? - warn "This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`." - end - - if ee_changes.any? && changelog.ee_changelog? && changelog.required_reasons.include?(:db_changes) - warn "This MR has a Changelog file inside `ee/`, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog placement to be outside of `ee/`. Consider moving the Changelog file outside `ee/`." - end -end - -if git.modified_files.include?("CHANGELOG.md") - fail changelog.modified_text -end - -changelog_found = changelog.found - -if changelog_found - check_changelog_yaml(changelog_found) - check_changelog_path(changelog_found) -elsif changelog.required? - changelog.required_texts.each { |_, text| fail(text) } # rubocop:disable Lint/UnreachableLoop -elsif changelog.optional? - message changelog.optional_text -end - -if helper.ci? && (changelog.required? || changelog.optional?) - checked = 0 - - git.commits.each do |commit| - case check_changelog_trailer(commit) - when :valid, :invalid - checked += 1 - end - end - - if checked == 0 - message <<~MSG - We are in the process of rolling out a new workflow for adding changelog entries. This new workflow uses Git commit subjects and Git trailers to generate changelogs. This new approach will soon replace the current YAML based approach. - - To ease the transition process, we recommend you start using both the old and new approach in parallel. This is not required at this time, but will make it easier to transition to the new approach in the future. To do so, pick the commit that should go in the changelog and add a `Changelog` trailer to it. For example: - - ``` - This is my commit's subject line - - This is the optional commit body. - - Changelog: added - ``` - - The value of the `Changelog` trailer should be one of the following: added, fixed, changed, deprecated, removed, security, performance, other. - - For more information, take a look at the following resources: - - - `https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/1564` - - https://docs.gitlab.com/ee/api/repositories.html#generate-changelog-data - - If you'd like to see the new approach in action, take a look at the commits in [the Omnibus repository](https://gitlab.com/gitlab-org/omnibus-gitlab/-/commits/master). - MSG - end -end +changelog.check! diff --git a/db/migrate/20210503131747_add_web_hook_calls_to_plan_limits.rb b/db/migrate/20210503131747_add_web_hook_calls_to_plan_limits.rb new file mode 100644 index 00000000000..cb23c9391ea --- /dev/null +++ b/db/migrate/20210503131747_add_web_hook_calls_to_plan_limits.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddWebHookCallsToPlanLimits < ActiveRecord::Migration[6.0] + def change + add_column :plan_limits, :web_hook_calls, :integer, null: false, default: 0 + end +end diff --git a/db/schema_migrations/20210503131747 b/db/schema_migrations/20210503131747 new file mode 100644 index 00000000000..52771dcb5de --- /dev/null +++ b/db/schema_migrations/20210503131747 @@ -0,0 +1 @@ +583c350d82c4d02e910f2c16ed2ec55ccdc880c87b55bf7bd6be3e1839958732
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index f04f7231947..1337d16a635 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16106,7 +16106,8 @@ CREATE TABLE plan_limits ( terraform_module_max_file_size bigint DEFAULT 1073741824 NOT NULL, helm_max_file_size bigint DEFAULT 5242880 NOT NULL, ci_registered_group_runners integer DEFAULT 1000 NOT NULL, - ci_registered_project_runners integer DEFAULT 1000 NOT NULL + ci_registered_project_runners integer DEFAULT 1000 NOT NULL, + web_hook_calls integer DEFAULT 0 NOT NULL ); CREATE SEQUENCE plan_limits_id_seq diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index 044a7403e8b..9ea76ff6151 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -112,6 +112,49 @@ Limit the maximum daily member invitations allowed per group hierarchy. - GitLab.com: Free members may invite 20 members per day. - Self-managed: Invites are not limited. +### Webhook calls + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61151) in GitLab 13.12. +> - [Deployed behind a feature flag](../user/feature_flags.md), disabled by default. +> - Disabled on GitLab.com. +> - Not recommended for production use. +> - To use in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-rate-limiting-for-webhooks). **(FREE SELF)** + +Limit the number of times any given webhook can be called per minute. +This only applies to project and group webhooks. + +Calls over the rate limit are logged into `auth.log`. + +```ruby +# If limits don't exist for the default plan, you can create one with: +# Plan.default.create_limits! + +Plan.default.actual_limits.update!(web_hook_calls: 10) +``` + +Set the limit to `0` to disable it. + +- **Default rate limit**: Disabled. + +#### Enable or disable rate limiting for webhooks **(FREE SELF)** + +Rate limiting for webhooks is under development and not ready for production use. It is +deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../administration/feature_flags.md) +can enable it. + +To enable it: + +```ruby +Feature.enable(:web_hooks_rate_limit) +``` + +To disable it: + +```ruby +Feature.disable(:web_hooks_rate_limit) +``` + ## Gitaly concurrency limit Clone traffic can put a large strain on your Gitaly service. To prevent such workloads from overwhelming your Gitaly server, you can set concurrency limits in Gitaly's configuration file. diff --git a/doc/ci/pipeline_editor/index.md b/doc/ci/pipeline_editor/index.md index 130e21502ed..1527a05d3d7 100644 --- a/doc/ci/pipeline_editor/index.md +++ b/doc/ci/pipeline_editor/index.md @@ -15,6 +15,7 @@ the `.gitlab-ci.yml` file in the root of your repository. To access the editor, From the pipeline editor page you can: +- Select the branch to work from. [Introduced in GitLab 13.12](https://gitlab.com/gitlab-org/gitlab/-/issues/326189), disabled by default. - [Validate](#validate-ci-configuration) your configuration syntax while editing the file. - Do a deeper [lint](#lint-ci-configuration) of your configuration, that verifies it with any configuration added with the [`include`](../yaml/README.md#include) keyword. diff --git a/doc/user/project/img/time_tracking_report_v13_12.png b/doc/user/project/img/time_tracking_report_v13_12.png Binary files differnew file mode 100644 index 00000000000..2132ca01cf4 --- /dev/null +++ b/doc/user/project/img/time_tracking_report_v13_12.png diff --git a/doc/user/project/img/time_tracking_sidebar_v13_12.png b/doc/user/project/img/time_tracking_sidebar_v13_12.png Binary files differnew file mode 100644 index 00000000000..e25282bc551 --- /dev/null +++ b/doc/user/project/img/time_tracking_sidebar_v13_12.png diff --git a/doc/user/project/img/time_tracking_sidebar_v8_16.png b/doc/user/project/img/time_tracking_sidebar_v8_16.png Binary files differdeleted file mode 100644 index 22124afed6f..00000000000 --- a/doc/user/project/img/time_tracking_sidebar_v8_16.png +++ /dev/null diff --git a/doc/user/project/time_tracking.md b/doc/user/project/time_tracking.md index 44bffe763e6..df76c4682f3 100644 --- a/doc/user/project/time_tracking.md +++ b/doc/user/project/time_tracking.md @@ -20,13 +20,14 @@ Time Tracking allows you to: - Record the time spent working on an issue or a merge request. - Add an estimate of the amount of time needed to complete an issue or a merge request. +- View a breakdown of time spent working on an issue or a merge request. You don't have to indicate an estimate to enter the time spent, and vice versa. Data about time tracking is shown on the issue/merge request sidebar, as shown below. - + ## How to enter data @@ -75,6 +76,19 @@ command fails and no time is logged. To remove all the time spent at once, use `/remove_time_spent`. +## View a time tracking report + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/271409) in GitLab 13.12. + +You can view a breakdown of time spent on an issue or merge request. + +To view a time tracking report, go to an issue or a merge request and select **Time tracking report** +in the right sidebar. + + + +The breakdown of spent time is limited to a maximum of 100 entries. + ## Configuration The following time units are available: diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index f74edf2b767..f91a56a0cd2 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -34,6 +34,7 @@ module Gitlab group_import: { threshold: -> { application_settings.group_import_limit }, interval: 1.minute }, group_testing_hook: { threshold: 5, interval: 1.minute }, profile_add_new_email: { threshold: 5, interval: 1.minute }, + web_hook_calls: { interval: 1.minute }, profile_resend_email_confirmation: { threshold: 5, interval: 1.minute }, update_environment_canary_ingress: { threshold: 1, interval: 1.minute }, auto_rollback_deployment: { threshold: 1, interval: 3.minutes } diff --git a/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml index 9b3a8e30da0..abcb347b146 100644 --- a/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml @@ -1,10 +1,11 @@ build: stage: build - image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-build-image:v0.4.0" + image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-build-image:v0.6.0" variables: DOCKER_TLS_CERTDIR: "" services: - - docker:19.03.12-dind + - name: "docker:20.10.6-dind" + command: ['--tls=false', '--host=tcp://0.0.0.0:2375'] script: - | if [[ -z "$CI_COMMIT_TAG" ]]; then diff --git a/lib/sidebars/projects/menus/issues_menu.rb b/lib/sidebars/projects/menus/issues_menu.rb index 4e8f9ec2b69..9840f644179 100644 --- a/lib/sidebars/projects/menus/issues_menu.rb +++ b/lib/sidebars/projects/menus/issues_menu.rb @@ -98,6 +98,10 @@ module Sidebars end def labels_menu_item + if Feature.enabled?(:sidebar_refactor, context.current_user) + return ::Sidebars::NilMenuItem.new(item_id: :labels) + end + ::Sidebars::MenuItem.new( title: _('Labels'), link: project_labels_path(context.project), diff --git a/lib/sidebars/projects/menus/labels_menu.rb b/lib/sidebars/projects/menus/labels_menu.rb index 0259839c43f..12cf0444994 100644 --- a/lib/sidebars/projects/menus/labels_menu.rb +++ b/lib/sidebars/projects/menus/labels_menu.rb @@ -40,6 +40,8 @@ module Sidebars override :render? def render? + return false if Feature.enabled?(:sidebar_refactor, context.current_user) + can?(context.current_user, :read_label, context.project) && !context.project.issues_enabled? end end diff --git a/lib/sidebars/projects/menus/project_information_menu.rb b/lib/sidebars/projects/menus/project_information_menu.rb index c78f6843ec1..82b0971f747 100644 --- a/lib/sidebars/projects/menus/project_information_menu.rb +++ b/lib/sidebars/projects/menus/project_information_menu.rb @@ -9,6 +9,7 @@ module Sidebars add_item(details_menu_item) add_item(activity_menu_item) add_item(releases_menu_item) + add_item(labels_menu_item) true end @@ -95,6 +96,19 @@ module Sidebars container_html_options: { class: 'shortcuts-project-releases' } ) end + + def labels_menu_item + if Feature.disabled?(:sidebar_refactor, context.current_user) + return ::Sidebars::NilMenuItem.new(item_id: :labels) + end + + ::Sidebars::MenuItem.new( + title: _('Labels'), + link: project_labels_path(context.project), + active_routes: { controller: :labels }, + item_id: :labels + ) + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index eccfdaec836..165a6ed4002 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18761,6 +18761,9 @@ msgstr "" msgid "Jobs|Create CI/CD configuration file" msgstr "" +msgid "Jobs|Job is stuck. Check runners." +msgstr "" + msgid "Jobs|Jobs are the building blocks of a GitLab CI/CD pipeline. Each job has a specific task, like testing code. To set up jobs in a CI/CD pipeline, add a CI/CD configuration file to your project." msgstr "" @@ -30676,6 +30679,9 @@ msgstr "" msgid "Speed up your pipelines with Needs relationships" msgstr "" +msgid "Spent At" +msgstr "" + msgid "Squash commit message" msgstr "" @@ -33574,6 +33580,9 @@ msgstr "" msgid "Time" msgstr "" +msgid "Time Spent" +msgstr "" + msgid "Time based: Yes" msgstr "" @@ -33625,6 +33634,9 @@ msgstr "" msgid "Time tracking" msgstr "" +msgid "Time tracking report" +msgstr "" + msgid "Time until first merge request" msgstr "" diff --git a/spec/features/projects/active_tabs_spec.rb b/spec/features/projects/active_tabs_spec.rb index 68695840808..96a321037a9 100644 --- a/spec/features/projects/active_tabs_spec.rb +++ b/spec/features/projects/active_tabs_spec.rb @@ -69,20 +69,37 @@ RSpec.describe 'Project active tab' do end context 'on project Issues' do + let(:feature_flag_value) { true } + before do + stub_feature_flags(sidebar_refactor: feature_flag_value) + visit project_issues_path(project) end it_behaves_like 'page has active tab', 'Issues' - %w(Milestones Labels).each do |sub_menu| - context "on project Issues/#{sub_menu}" do - before do - click_tab(sub_menu) - end + context "on project Issues/Milestones" do + before do + click_tab('Milestones') + end - it_behaves_like 'page has active tab', 'Issues' - it_behaves_like 'page has active sub tab', sub_menu + it_behaves_like 'page has active tab', 'Issues' + it_behaves_like 'page has active sub tab', 'Milestones' + end + + context 'when feature flag is disabled' do + let(:feature_flag_value) { false } + + %w(Milestones Labels).each do |sub_menu| + context "on project Issues/#{sub_menu}" do + before do + click_tab(sub_menu) + end + + it_behaves_like 'page has active tab', 'Issues' + it_behaves_like 'page has active sub tab', sub_menu + end end end end diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index ab82a4750d3..363fe8c35fe 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -54,17 +54,30 @@ RSpec.describe 'Edit Project Settings' do end context 'When external issue tracker is enabled and issues disabled on project settings' do - it 'hides issues tab and show labels tab' do + before do project.issues_enabled = false project.save! allow_next_instance_of(Project) do |instance| allow(instance).to receive(:external_issue_tracker).and_return(JiraService.new) end + end + it 'hides issues tab' do visit project_path(project) expect(page).not_to have_selector('.shortcuts-issues') - expect(page).to have_selector('.shortcuts-labels') + expect(page).not_to have_selector('.shortcuts-labels') + end + + context 'when feature flag :sidebar_refactor is disabled' do + it 'hides issues tab and show labels tab' do + stub_feature_flags(sidebar_refactor: false) + + visit project_path(project) + + expect(page).not_to have_selector('.shortcuts-issues') + expect(page).to have_selector('.shortcuts-labels') + end end end diff --git a/spec/features/projects/navbar_spec.rb b/spec/features/projects/navbar_spec.rb index 662bbd9b26f..74f1c639af3 100644 --- a/spec/features/projects/navbar_spec.rb +++ b/spec/features/projects/navbar_spec.rb @@ -8,70 +8,81 @@ RSpec.describe 'Project navbar' do include_context 'project navbar structure' - let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository) } - before do - stub_feature_flags(sidebar_refactor: false) - insert_package_nav(_('Operations')) - insert_infrastructure_registry_nav - stub_config(registry: { enabled: false }) + let(:user) { project.owner } - project.add_maintainer(user) + before do sign_in(user) end - it_behaves_like 'verified navigation bar' do + context 'when sidebar refactor feature flag is disabled' do before do - visit project_path(project) + stub_feature_flags(sidebar_refactor: false) + insert_package_nav(_('Operations')) + insert_infrastructure_registry_nav + + insert_after_sub_nav_item( + _('Boards'), + within: _('Issues'), + new_sub_nav_item_name: _('Labels') + ) + + stub_config(registry: { enabled: false }) end - end - context 'when value stream is available' do - before do - visit project_path(project) + it_behaves_like 'verified navigation bar' do + before do + visit project_path(project) + end end - it 'redirects to value stream when Analytics item is clicked' do - page.within('.sidebar-top-level-items') do - find('.shortcuts-analytics').click + context 'when value stream is available' do + before do + visit project_path(project) end - wait_for_requests + it 'redirects to value stream when Analytics item is clicked' do + page.within('.sidebar-top-level-items') do + find('.shortcuts-analytics').click + end + + wait_for_requests - expect(page).to have_current_path(project_cycle_analytics_path(project)) + expect(page).to have_current_path(project_cycle_analytics_path(project)) + end end - end - context 'when pages are available' do - before do - stub_config(pages: { enabled: true }) + context 'when pages are available' do + before do + stub_config(pages: { enabled: true }) - insert_after_sub_nav_item( - _('Operations'), - within: _('Settings'), - new_sub_nav_item_name: _('Pages') - ) + insert_after_sub_nav_item( + _('Operations'), + within: _('Settings'), + new_sub_nav_item_name: _('Pages') + ) - visit project_path(project) + visit project_path(project) + end + + it_behaves_like 'verified navigation bar' end - it_behaves_like 'verified navigation bar' - end + context 'when container registry is available' do + before do + stub_config(registry: { enabled: true }) - context 'when container registry is available' do - before do - stub_config(registry: { enabled: true }) + insert_container_nav - insert_container_nav + visit project_path(project) + end - visit project_path(project) + it_behaves_like 'verified navigation bar' end - - it_behaves_like 'verified navigation bar' end - context 'when sidebar refactor feature flag is on' do + context 'when sidebar refactor feature flag is enabled' do let(:operations_menu_items) do [ _('Metrics'), @@ -91,7 +102,8 @@ RSpec.describe 'Project navbar' do nav_item: _('Project information'), nav_sub_items: [ _('Activity'), - _('Releases') + _('Releases'), + _('Labels') ] } end @@ -99,7 +111,8 @@ RSpec.describe 'Project navbar' do before do stub_feature_flags(sidebar_refactor: true) stub_config(registry: { enabled: true }) - + insert_package_nav(_('Operations')) + insert_infrastructure_registry_nav insert_container_nav insert_after_sub_nav_item( diff --git a/spec/frontend/jobs/components/table/cells.vue/job_cell_spec.js b/spec/frontend/jobs/components/table/cells.vue/job_cell_spec.js index 77c19162747..fc4e5586349 100644 --- a/spec/frontend/jobs/components/table/cells.vue/job_cell_spec.js +++ b/spec/frontend/jobs/components/table/cells.vue/job_cell_spec.js @@ -7,6 +7,7 @@ import { mockJobsInTable } from '../../../mock_data'; const mockJob = mockJobsInTable[0]; const mockJobCreatedByTag = mockJobsInTable[1]; const mockJobLimitedAccess = mockJobsInTable[2]; +const mockStuckJob = mockJobsInTable[3]; describe('Job Cell', () => { let wrapper; @@ -17,6 +18,7 @@ describe('Job Cell', () => { const findJobSha = () => wrapper.findByTestId('job-sha'); const findLabelIcon = () => wrapper.findByTestId('label-icon'); const findForkIcon = () => wrapper.findByTestId('fork-icon'); + const findStuckIcon = () => wrapper.findByTestId('stuck-icon'); const findAllTagBadges = () => wrapper.findAllByTestId('job-tag-badge'); const findBadgeById = (id) => wrapper.findByTestId(id); @@ -120,4 +122,19 @@ describe('Job Cell', () => { expect(findBadgeById(testId).text()).toBe(text); }); }); + + describe('Job icons', () => { + it('stuck icon is not shown if job is not stuck', () => { + createComponent(); + + expect(findStuckIcon().exists()).toBe(false); + }); + + it('stuck icon is shown if job is stuck', () => { + createComponent(mockStuckJob); + + expect(findStuckIcon().exists()).toBe(true); + expect(findStuckIcon().attributes('name')).toBe('warning'); + }); + }); }); diff --git a/spec/frontend/jobs/mock_data.js b/spec/frontend/jobs/mock_data.js index 6d9a5b77f5a..7851f633629 100644 --- a/spec/frontend/jobs/mock_data.js +++ b/spec/frontend/jobs/mock_data.js @@ -1322,6 +1322,7 @@ export const mockJobsInTable = [ playable: true, cancelable: false, active: false, + stuck: false, userPermissions: { readBuild: true, __typename: 'JobPermissions' }, __typename: 'CiJob', }, @@ -1361,6 +1362,7 @@ export const mockJobsInTable = [ playable: false, cancelable: false, active: false, + stuck: false, userPermissions: { readBuild: true, __typename: 'JobPermissions' }, __typename: 'CiJob', }, @@ -1407,9 +1409,65 @@ export const mockJobsInTable = [ playable: false, cancelable: false, active: false, + stuck: false, userPermissions: { readBuild: false, __typename: 'JobPermissions' }, __typename: 'CiJob', }, + { + artifacts: { nodes: [], __typename: 'CiJobArtifactConnection' }, + allowFailure: false, + status: 'PENDING', + scheduledAt: null, + manualJob: false, + triggered: null, + createdByTag: false, + detailedStatus: { + detailsPath: '/root/ci-project/-/jobs/2391', + group: 'pending', + icon: 'status_pending', + label: 'pending', + text: 'pending', + tooltip: 'pending', + action: { + buttonTitle: 'Cancel this job', + icon: 'cancel', + method: 'post', + path: '/root/ci-project/-/jobs/2391/cancel', + title: 'Cancel', + __typename: 'StatusAction', + }, + __typename: 'DetailedStatus', + }, + id: 'gid://gitlab/Ci::Build/2391', + refName: 'master', + refPath: '/root/ci-project/-/commits/master', + tags: [], + shortSha: '916330b4', + commitPath: '/root/ci-project/-/commit/916330b4fda5dae226524ceb51c756c0ed26679d', + pipeline: { + id: 'gid://gitlab/Ci::Pipeline/482', + path: '/root/ci-project/-/pipelines/482', + user: { + webPath: '/root', + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + __typename: 'UserCore', + }, + __typename: 'Pipeline', + }, + stage: { name: 'build', __typename: 'CiStage' }, + name: 'build_job', + duration: null, + finishedAt: null, + coverage: null, + retryable: false, + playable: false, + cancelable: true, + active: true, + stuck: true, + userPermissions: { readBuild: true, __typename: 'JobPermissions' }, + __typename: 'CiJob', + }, ]; export const mockJobsQueryResponse = { @@ -1495,6 +1553,7 @@ export const mockJobsQueryResponse = { playable: false, cancelable: false, active: false, + stuck: false, userPermissions: { readBuild: true, __typename: 'JobPermissions' }, __typename: 'CiJob', }, diff --git a/spec/frontend/sidebar/components/time_tracking/mock_data.js b/spec/frontend/sidebar/components/time_tracking/mock_data.js new file mode 100644 index 00000000000..862bcbe861e --- /dev/null +++ b/spec/frontend/sidebar/components/time_tracking/mock_data.js @@ -0,0 +1,102 @@ +export const getIssueTimelogsQueryResponse = { + data: { + issuable: { + __typename: 'Issue', + id: 'gid://gitlab/Issue/148', + title: + 'Est perferendis dicta expedita ipsum adipisci laudantium omnis consequatur consequatur et.', + timelogs: { + nodes: [ + { + __typename: 'Timelog', + timeSpent: 14400, + user: { + name: 'John Doe18', + __typename: 'UserCore', + }, + spentAt: '2020-05-01T00:00:00Z', + note: { + body: 'I paired with @root on this last week.', + __typename: 'Note', + }, + }, + { + __typename: 'Timelog', + timeSpent: 1800, + user: { + name: 'Administrator', + __typename: 'UserCore', + }, + spentAt: '2021-05-07T13:19:01Z', + note: null, + }, + { + __typename: 'Timelog', + timeSpent: 14400, + user: { + name: 'Administrator', + __typename: 'UserCore', + }, + spentAt: '2021-05-01T00:00:00Z', + note: { + body: 'I did some work on this last week.', + __typename: 'Note', + }, + }, + ], + __typename: 'TimelogConnection', + }, + }, + }, +}; + +export const getMrTimelogsQueryResponse = { + data: { + issuable: { + __typename: 'MergeRequest', + id: 'gid://gitlab/MergeRequest/29', + title: 'Esse amet perspiciatis voluptas et sed praesentium debitis repellat.', + timelogs: { + nodes: [ + { + __typename: 'Timelog', + timeSpent: 1800, + user: { + name: 'Administrator', + __typename: 'UserCore', + }, + spentAt: '2021-05-07T14:44:55Z', + note: { + body: 'Thirty minutes!', + __typename: 'Note', + }, + }, + { + __typename: 'Timelog', + timeSpent: 3600, + user: { + name: 'Administrator', + __typename: 'UserCore', + }, + spentAt: '2021-05-07T14:44:39Z', + note: null, + }, + { + __typename: 'Timelog', + timeSpent: 300, + user: { + name: 'Administrator', + __typename: 'UserCore', + }, + spentAt: '2021-03-10T00:00:00Z', + note: { + body: 'A note with some time', + __typename: 'Note', + }, + }, + ], + __typename: 'TimelogConnection', + }, + }, + }, +}; diff --git a/spec/frontend/sidebar/components/time_tracking/report_spec.js b/spec/frontend/sidebar/components/time_tracking/report_spec.js new file mode 100644 index 00000000000..603baef4a5f --- /dev/null +++ b/spec/frontend/sidebar/components/time_tracking/report_spec.js @@ -0,0 +1,97 @@ +import { GlLoadingIcon } from '@gitlab/ui'; +import { getAllByRole } from '@testing-library/dom'; +import { shallowMount, createLocalVue, mount } from '@vue/test-utils'; +import VueApollo from 'vue-apollo'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import createFlash from '~/flash'; +import Report from '~/sidebar/components/time_tracking/report.vue'; +import getIssueTimelogsQuery from '~/vue_shared/components/sidebar/queries/get_issue_timelogs.query.graphql'; +import getMrTimelogsQuery from '~/vue_shared/components/sidebar/queries/get_mr_timelogs.query.graphql'; +import { getIssueTimelogsQueryResponse, getMrTimelogsQueryResponse } from './mock_data'; + +jest.mock('~/flash'); + +describe('Issuable Time Tracking Report', () => { + const localVue = createLocalVue(); + localVue.use(VueApollo); + let wrapper; + let fakeApollo; + const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const successIssueQueryHandler = jest.fn().mockResolvedValue(getIssueTimelogsQueryResponse); + const successMrQueryHandler = jest.fn().mockResolvedValue(getMrTimelogsQueryResponse); + + const mountComponent = ({ + queryHandler = successIssueQueryHandler, + issuableType = 'issue', + mountFunction = shallowMount, + } = {}) => { + fakeApollo = createMockApollo([ + [getIssueTimelogsQuery, queryHandler], + [getMrTimelogsQuery, queryHandler], + ]); + wrapper = mountFunction(Report, { + provide: { + issuableId: 1, + issuableType, + }, + localVue, + apolloProvider: fakeApollo, + }); + }; + + afterEach(() => { + wrapper.destroy(); + fakeApollo = null; + }); + + it('should render loading spinner', () => { + mountComponent(); + + expect(findLoadingIcon()).toExist(); + }); + + it('should render error message on reject', async () => { + mountComponent({ queryHandler: jest.fn().mockRejectedValue('ERROR') }); + await waitForPromises(); + + expect(createFlash).toHaveBeenCalled(); + }); + + describe('for issue', () => { + beforeEach(() => { + mountComponent({ mountFunction: mount }); + }); + + it('calls correct query', () => { + expect(successIssueQueryHandler).toHaveBeenCalled(); + }); + + it('renders correct results', async () => { + await waitForPromises(); + + expect(getAllByRole(wrapper.element, 'row', { name: /John Doe18/i })).toHaveLength(1); + expect(getAllByRole(wrapper.element, 'row', { name: /Administrator/i })).toHaveLength(2); + }); + }); + + describe('for merge request', () => { + beforeEach(() => { + mountComponent({ + queryHandler: successMrQueryHandler, + issuableType: 'merge_request', + mountFunction: mount, + }); + }); + + it('calls correct query', () => { + expect(successMrQueryHandler).toHaveBeenCalled(); + }); + + it('renders correct results', async () => { + await waitForPromises(); + + expect(getAllByRole(wrapper.element, 'row', { name: /Administrator/i })).toHaveLength(3); + }); + }); +}); diff --git a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js index 4d03aedf1be..f26cdcb8b20 100644 --- a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js +++ b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js @@ -10,6 +10,7 @@ describe('Issuable Time Tracker', () => { const findComparisonMeter = () => findByTestId('compareMeter').attributes('title'); const findCollapsedState = () => findByTestId('collapsedState'); const findTimeRemainingProgress = () => findByTestId('timeRemainingProgress'); + const findReportLink = () => findByTestId('reportLink'); const defaultProps = { timeEstimate: 10_000, // 2h 46m @@ -192,6 +193,33 @@ describe('Issuable Time Tracker', () => { }); }); + describe('Time tracking report', () => { + describe('When no time spent', () => { + beforeEach(() => { + wrapper = mountComponent({ + props: { + timeSpent: 0, + timeSpentHumanReadable: '', + }, + }); + }); + + it('link should not appear', () => { + expect(findReportLink().exists()).toBe(false); + }); + }); + + describe('When time spent', () => { + beforeEach(() => { + wrapper = mountComponent(); + }); + + it('link should appear', () => { + expect(findReportLink().exists()).toBe(true); + }); + }); + }); + describe('Help pane', () => { const findHelpButton = () => findByTestId('helpButton'); const findCloseHelpButton = () => findByTestId('closeHelpButton'); diff --git a/spec/frontend/task_list_spec.js b/spec/frontend/task_list_spec.js index b6ac3167fea..2d7a735bd11 100644 --- a/spec/frontend/task_list_spec.js +++ b/spec/frontend/task_list_spec.js @@ -16,7 +16,20 @@ describe('TaskList', () => { beforeEach(() => { setFixtures(` <div class="task-list"> - <div class="js-task-list-container"></div> + <div class="js-task-list-container"> + <ul data-sourcepos="5:1-5:11" class="task-list" dir="auto"> + <li data-sourcepos="5:1-5:11" class="task-list-item enabled"> + <input type="checkbox" class="task-list-item-checkbox" checked=""> markdown task + </li> + </ul> + + <ul class="task-list" dir="auto"> + <li class="task-list-item enabled"> + <input type="checkbox" class="task-list-item-checkbox"> hand-coded checkbox + </li> + </ul> + <textarea class="hidden js-task-list-field"></textarea> + </div> </div> `); @@ -59,32 +72,47 @@ describe('TaskList', () => { describe('disableTaskListItems', () => { it('should call taskList method with disable param', () => { - jest.spyOn($.prototype, 'taskList').mockImplementation(() => {}); + taskList.disableTaskListItems(); - taskList.disableTaskListItems({ currentTarget }); - - expect(currentTarget.taskList).toHaveBeenCalledWith('disable'); + expect(document.querySelectorAll('.task-list-item input:disabled').length).toEqual(2); }); }); describe('enableTaskListItems', () => { - it('should call taskList method with enable param', () => { - jest.spyOn($.prototype, 'taskList').mockImplementation(() => {}); + it('should enable markdown tasks and disable non-markdown tasks', () => { + taskList.disableTaskListItems(); + taskList.enableTaskListItems(); + + expect(document.querySelectorAll('.task-list-item input:enabled').length).toEqual(1); + expect(document.querySelectorAll('.task-list-item input:disabled').length).toEqual(1); + }); + }); + + describe('enable', () => { + it('should enable task list items and on document event', () => { + jest.spyOn($.prototype, 'on').mockImplementation(() => {}); + + taskList.enable(); - taskList.enableTaskListItems({ currentTarget }); + expect(document.querySelectorAll('.task-list-item input:enabled').length).toEqual(1); + expect(document.querySelectorAll('.task-list-item input:disabled').length).toEqual(1); - expect(currentTarget.taskList).toHaveBeenCalledWith('enable'); + expect($(document).on).toHaveBeenCalledWith( + 'tasklist:changed', + taskList.taskListContainerSelector, + taskList.updateHandler, + ); }); }); describe('disable', () => { it('should disable task list items and off document event', () => { - jest.spyOn(taskList, 'disableTaskListItems').mockImplementation(() => {}); jest.spyOn($.prototype, 'off').mockImplementation(() => {}); taskList.disable(); - expect(taskList.disableTaskListItems).toHaveBeenCalled(); + expect(document.querySelectorAll('.task-list-item input:disabled').length).toEqual(2); + expect($(document).off).toHaveBeenCalledWith( 'tasklist:changed', taskList.taskListContainerSelector, diff --git a/spec/helpers/boards_helper_spec.rb b/spec/helpers/boards_helper_spec.rb index 00cd44809c7..cb4b6915b20 100644 --- a/spec/helpers/boards_helper_spec.rb +++ b/spec/helpers/boards_helper_spec.rb @@ -36,7 +36,7 @@ RSpec.describe BoardsHelper do end describe '#board_base_url' do - context 'when project board' do + context 'when group board' do it 'generates the correct url' do assign(:board, group_board) assign(:group, base_group) @@ -55,6 +55,43 @@ RSpec.describe BoardsHelper do end end + describe '#current_board_namespace' do + context 'when group board' do + it 'returns the correct namespace' do + assign(:board, group_board) + assign(:group, base_group) + + expect(helper.current_board_namespace).to be(base_group) + end + end + + context 'project under group' do + context 'when project board' do + it 'returns the correct namespace' do + assign(:project, project) + assign(:board, project_board) + + expect(helper.current_board_namespace).to be(project.parent) + end + end + end + + context 'project under user namespace' do + let_it_be(:project_under_user) { create(:project, namespace: user.namespace) } + + context 'when project board' do + let_it_be(:project_board) { create(:board, project: project_under_user) } + + it 'returns the correct namespace' do + assign(:project, project_under_user) + assign(:board, project_board) + + expect(helper.current_board_namespace).to be(user.namespace) + end + end + end + end + describe '#board_data' do context 'project_board' do before do diff --git a/spec/lib/sidebars/projects/menus/issues_menu_spec.rb b/spec/lib/sidebars/projects/menus/issues_menu_spec.rb index e5d486bbe8f..ac62cd7594a 100644 --- a/spec/lib/sidebars/projects/menus/issues_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/issues_menu_spec.rb @@ -65,4 +65,22 @@ RSpec.describe Sidebars::Projects::Menus::IssuesMenu do end end end + + describe 'Menu Items' do + subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } } + + describe 'Labels' do + let(:item_id) { :labels } + + specify { is_expected.to be_nil } + + context 'when feature flag :sidebar_refactor is disabled' do + before do + stub_feature_flags(sidebar_refactor: false) + end + + specify { is_expected.not_to be_nil } + end + end + end end diff --git a/spec/lib/sidebars/projects/menus/labels_menu_spec.rb b/spec/lib/sidebars/projects/menus/labels_menu_spec.rb index 588119746cf..e1420f9e61b 100644 --- a/spec/lib/sidebars/projects/menus/labels_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/labels_menu_spec.rb @@ -20,27 +20,41 @@ RSpec.describe Sidebars::Projects::Menus::LabelsMenu do allow(project).to receive(:issues_enabled?).and_return(issues_enabled) end - context 'when user can read labels' do - context 'when issues feature is enabled' do - it 'returns false' do - expect(subject.render?).to be_falsey - end + context 'when feature flag :sidebar_refactor is enabled' do + let(:issues_enabled) { false } + + it 'returns false' do + expect(subject.render?).to be_falsey end + end - context 'when issues feature is disabled' do - let(:issues_enabled) { false } + context 'when feature flag :sidebar_refactor is disabled' do + before do + stub_feature_flags(sidebar_refactor: false) + end - it 'returns true' do - expect(subject.render?).to be_truthy + context 'when user can read labels' do + context 'when issues feature is enabled' do + it 'returns false' do + expect(subject.render?).to be_falsey + end + end + + context 'when issues feature is disabled' do + let(:issues_enabled) { false } + + it 'returns true' do + expect(subject.render?).to be_truthy + end end end - end - context 'when user cannot read labels' do - let(:user) { nil } + context 'when user cannot read labels' do + let(:user) { nil } - it 'returns false' do - expect(subject.render?).to be_falsey + it 'returns false' do + expect(subject.render?).to be_falsey + end end end end diff --git a/spec/lib/sidebars/projects/menus/project_information_menu_spec.rb b/spec/lib/sidebars/projects/menus/project_information_menu_spec.rb index ed162910bdd..d58766ae5e0 100644 --- a/spec/lib/sidebars/projects/menus/project_information_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/project_information_menu_spec.rb @@ -8,30 +8,48 @@ RSpec.describe Sidebars::Projects::Menus::ProjectInformationMenu do let(:user) { project.owner } let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) } - describe 'Releases' do - subject { described_class.new(context).renderable_items.index { |e| e.item_id == :releases } } + describe 'Menu Items' do + subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } } - context 'when project repository is empty' do - it 'does not include releases menu item' do - allow(project).to receive(:empty_repo?).and_return(true) + describe 'Releases' do + let(:item_id) { :releases } - is_expected.to be_nil + context 'when project repository is empty' do + it 'does not include releases menu item' do + allow(project).to receive(:empty_repo?).and_return(true) + + is_expected.to be_nil + end end - end - context 'when project repository is not empty' do - context 'when user can download code' do - it 'includes releases menu item' do - is_expected.to be_present + context 'when project repository is not empty' do + context 'when user can download code' do + it 'includes releases menu item' do + is_expected.to be_present + end + end + + context 'when user cannot download code' do + let(:user) { nil } + + it 'does not include releases menu item' do + is_expected.to be_nil + end end end + end - context 'when user cannot download code' do - let(:user) { nil } + describe 'Labels' do + let(:item_id) { :labels } - it 'does not include releases menu item' do - is_expected.to be_nil + specify { is_expected.not_to be_nil } + + context 'when feature flag :sidebar_refactor is disabled' do + before do + stub_feature_flags(sidebar_refactor: false) end + + specify { is_expected.to be_nil } end end end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index 69fbc4c3b4f..88149465232 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -30,4 +30,13 @@ RSpec.describe ProjectHook do expect(described_class.tag_push_hooks).to eq([hook]) end end + + describe '#rate_limit' do + let_it_be(:hook) { create(:project_hook) } + let_it_be(:plan_limits) { create(:plan_limits, :default_plan, web_hook_calls: 100) } + + it 'returns the default limit' do + expect(hook.rate_limit).to be(100) + end + end end diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 00ec5a1d9b9..651716c3280 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -22,4 +22,12 @@ RSpec.describe ServiceHook do hook.execute(data) end end + + describe '#rate_limit' do + let(:hook) { build(:service_hook) } + + it 'returns nil' do + expect(hook.rate_limit).to be_nil + end + end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 02e630cbf27..a72034f1ac5 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -169,4 +169,12 @@ RSpec.describe SystemHook do hook.async_execute(data, hook_name) end end + + describe '#rate_limit' do + let(:hook) { build(:system_hook) } + + it 'returns nil' do + expect(hook.rate_limit).to be_nil + end + end end diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index 4259c8b708b..b8c723b3847 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -210,6 +210,7 @@ RSpec.describe PlanLimits do ci_active_jobs storage_size_limit daily_invites + web_hook_calls ] + disabled_max_artifact_size_columns end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 696b0bcaaf2..b3fd4e33640 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -5,8 +5,9 @@ require 'spec_helper' RSpec.describe WebHookService do include StubRequests - let(:project) { create(:project) } - let(:project_hook) { create(:project_hook) } + let_it_be(:project) { create(:project) } + let_it_be_with_reload(:project_hook) { create(:project_hook, project: project) } + let(:headers) do { 'Content-Type' => 'application/json', @@ -60,12 +61,8 @@ RSpec.describe WebHookService do end describe '#execute' do - before do - project.hooks << [project_hook] - end - context 'when token is defined' do - let(:project_hook) { create(:project_hook, :token) } + let_it_be(:project_hook) { create(:project_hook, :token) } it 'POSTs to the webhook URL' do stub_full_request(project_hook.url, method: :post) @@ -89,8 +86,8 @@ RSpec.describe WebHookService do end context 'when auth credentials are present' do - let(:url) {'https://example.org'} - let(:project_hook) { create(:project_hook, url: 'https://demo:demo@example.org/') } + let_it_be(:url) {'https://example.org'} + let_it_be(:project_hook) { create(:project_hook, url: 'https://demo:demo@example.org/') } it 'uses the credentials' do stub_full_request(url, method: :post) @@ -104,8 +101,8 @@ RSpec.describe WebHookService do end context 'when auth credentials are partial present' do - let(:url) {'https://example.org'} - let(:project_hook) { create(:project_hook, url: 'https://demo@example.org/') } + let_it_be(:url) {'https://example.org'} + let_it_be(:project_hook) { create(:project_hook, url: 'https://demo@example.org/') } it 'uses the credentials anyways' do stub_full_request(url, method: :post) @@ -147,7 +144,7 @@ RSpec.describe WebHookService do end context 'when url is not encoded' do - let(:project_hook) { create(:project_hook, url: 'http://server.com/my path/') } + let_it_be(:project_hook) { create(:project_hook, url: 'http://server.com/my path/') } it 'handles exceptions' do expect(service_instance.execute).to eq(status: :error, message: 'bad URI(is not URI?): "http://server.com/my path/"') @@ -340,12 +337,98 @@ RSpec.describe WebHookService do end describe '#async_execute' do - let(:system_hook) { create(:system_hook) } + def expect_to_perform_worker(hook) + expect(WebHookWorker).to receive(:perform_async).with(hook.id, data, 'push_hooks') + end + + def expect_to_rate_limit(hook, threshold:, throttled: false) + expect(Gitlab::ApplicationRateLimiter).to receive(:throttled?) + .with(:web_hook_calls, scope: [hook], threshold: threshold) + .and_return(throttled) + end + + context 'when rate limiting is not configured' do + it 'queues a worker without tracking the call' do + expect(Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + expect_to_perform_worker(project_hook) + + service_instance.async_execute + end + end - it 'enqueue WebHookWorker' do - expect(WebHookWorker).to receive(:perform_async).with(project_hook.id, data, 'push_hooks') + context 'when rate limiting is configured' do + let_it_be(:threshold) { 3 } + let_it_be(:plan_limits) { create(:plan_limits, :default_plan, web_hook_calls: threshold) } - described_class.new(project_hook, data, 'push_hooks').async_execute + it 'queues a worker and tracks the call' do + expect_to_rate_limit(project_hook, threshold: threshold) + expect_to_perform_worker(project_hook) + + service_instance.async_execute + end + + context 'when the hook is throttled (via mock)' do + before do + expect_to_rate_limit(project_hook, threshold: threshold, throttled: true) + end + + it 'does not queue a worker and logs an error' do + expect(WebHookWorker).not_to receive(:perform_async) + + payload = { + message: 'Webhook rate limit exceeded', + hook_id: project_hook.id, + hook_type: 'ProjectHook', + hook_name: 'push_hooks' + } + + expect(Gitlab::AuthLogger).to receive(:error).with(payload) + expect(Gitlab::AppLogger).to receive(:error).with(payload) + + service_instance.async_execute + end + end + + context 'when the hook is throttled (via Redis)', :clean_gitlab_redis_cache do + before do + # Set a high interval to avoid intermittent failures in CI + allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits).and_return( + web_hook_calls: { interval: 1.day } + ) + + expect_to_perform_worker(project_hook).exactly(threshold).times + + threshold.times { service_instance.async_execute } + end + + it 'stops queueing workers and logs errors' do + expect(Gitlab::AuthLogger).to receive(:error).twice + expect(Gitlab::AppLogger).to receive(:error).twice + + 2.times { service_instance.async_execute } + end + + it 'still queues workers for other hooks' do + other_hook = create(:project_hook) + + expect_to_perform_worker(other_hook) + + described_class.new(other_hook, data, :push_hooks).async_execute + end + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(web_hooks_rate_limit: false) + end + + it 'queues a worker without tracking the call' do + expect(Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + expect_to_perform_worker(project_hook) + + service_instance.async_execute + end + end end end end diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index 52c8f2b5986..ab24828a057 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -73,7 +73,6 @@ RSpec.shared_context 'project navbar structure' do nav_sub_items: [ _('List'), _('Boards'), - _('Labels'), _('Service Desk'), _('Milestones'), (_('Iterations') if Gitlab.ee?) diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb index 7ea2288fd45..0a6af3ea798 100644 --- a/spec/tooling/danger/changelog_spec.rb +++ b/spec/tooling/danger/changelog_spec.rb @@ -18,6 +18,204 @@ RSpec.describe Tooling::Danger::Changelog do allow(changelog).to receive(:project_helper).and_return(fake_project_helper) end + describe '#check_changelog_trailer' do + subject { changelog.check_changelog_trailer(commit) } + + context "when commit doesn't include a changelog trailer" do + let(:commit) { double('commit', message: "Hello world") } + + it { is_expected.to be_nil } + end + + context "when commit include a changelog trailer with no category" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog:") } + + it { is_expected.to be_nil } + end + + context "when commit include a changelog trailer with an unknown category" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog: foo", sha: "abc123") } + + it { is_expected.to have_attributes(errors: ["Commit #{commit.sha} uses an invalid changelog category: foo"]) } + end + + described_class::CATEGORIES.each do |category| + context "when commit include a changelog trailer with category set to '#{category}'" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog: #{category}", sha: "abc123") } + + it { is_expected.to have_attributes(errors: []) } + end + end + end + + describe '#check_changelog_yaml' do + let(:changelog_path) { 'ee/changelogs/unreleased/entry.yml' } + let(:changes) { changes_class.new([change_class.new(changelog_path, :added, :changelog)]) } + let(:yaml_title) { 'Fix changelog Dangerfile to convert MR IID to a string before comparison' } + let(:yaml_merge_request) { 60899 } + let(:mr_iid) { '60899' } + let(:yaml_type) { 'fixed' } + let(:yaml) do + <<~YAML + --- + title: #{yaml_title} + merge_request: #{yaml_merge_request} + author: + type: #{yaml_type} + YAML + end + + before do + allow(changelog).to receive(:present?).and_return(true) + allow(changelog).to receive(:changelog_path).and_return(changelog_path) + allow(changelog).to receive(:read_file).with(changelog_path).and_return(yaml) + allow(fake_helper).to receive(:security_mr?).and_return(false) + allow(fake_helper).to receive(:mr_iid).and_return(mr_iid) + allow(fake_helper).to receive(:cherry_pick_mr?).and_return(false) + allow(fake_helper).to receive(:stable_branch?).and_return(false) + allow(fake_helper).to receive(:html_link).with(changelog_path).and_return(changelog_path) + end + + subject { changelog.check_changelog_yaml } + + context "when changelog is not present" do + before do + allow(changelog).to receive(:present?).and_return(false) + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "when YAML is invalid" do + let(:yaml) { '{ foo bar]' } + + it { is_expected.to have_attributes(errors: ["#{changelog_path} isn't valid YAML! #{described_class::SEE_DOC}"]) } + end + + context "when a StandardError is raised" do + before do + allow(changelog).to receive(:read_file).and_raise(StandardError, "Fail!") + end + + it { is_expected.to have_attributes(warnings: ["There was a problem trying to check the Changelog. Exception: StandardError - Fail!"]) } + end + + context "when YAML title is nil" do + let(:yaml_title) { '' } + + it { is_expected.to have_attributes(errors: ["`title` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) } + end + + context "when YAML type is nil" do + let(:yaml_type) { '' } + + it { is_expected.to have_attributes(errors: ["`type` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) } + end + + context "when on a security MR" do + let(:yaml_merge_request) { '' } + + before do + allow(fake_helper).to receive(:security_mr?).and_return(true) + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "when MR IID is empty" do + before do + allow(fake_helper).to receive(:mr_iid).and_return("") + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "when YAML MR IID is empty" do + let(:yaml_merge_request) { '' } + + context "and YAML includes a merge_request: line" do + it { is_expected.to have_attributes(markdowns: [{ msg: format(described_class::SUGGEST_MR_COMMENT, mr_iid: fake_helper.mr_iid), file: changelog_path, line: 3 }]) } + end + + context "and YAML does not include a merge_request: line" do + let(:yaml) do + <<~YAML + --- + title: #{yaml_title} + author: + type: #{yaml_type} + YAML + end + + it { is_expected.to have_attributes(messages: ["Consider setting `merge_request` to #{mr_iid} in #{changelog_path}. #{described_class::SEE_DOC}"]) } + end + end + end + + describe '#check_changelog_path' do + let(:changelog_path) { 'changelog-path.yml' } + let(:foss_change) { nil } + let(:ee_change) { nil } + let(:changelog_change) { nil } + let(:changes) { changes_class.new([foss_change, ee_change, changelog_change].compact) } + + before do + allow(changelog).to receive(:present?).and_return(true) + end + + subject { changelog.check_changelog_path } + + context "when changelog is not present" do + before do + allow(changelog).to receive(:present?).and_return(false) + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "with EE changes" do + let(:ee_change) { change_class.new('ee/app/models/foo.rb', :added, :backend) } + + context "and a non-EE changelog, and changelog not required" do + let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) } + + before do + allow(changelog).to receive(:required?).and_return(false) + end + + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`."]) } + end + + context "and a EE changelog" do + let(:changelog_change) { change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog) } + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + + context "and there are DB changes" do + let(:foss_change) { change_class.new('db/migrate/foo.rb', :added, :migration) } + + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file inside `ee/`, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog placement to be outside of `ee/`. Consider moving the Changelog file outside `ee/`."]) } + end + end + end + + context "with no EE changes" do + let(:foss_change) { change_class.new('app/models/foo.rb', :added, :backend) } + + context "and a non-EE changelog" do + let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) } + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + end + + context "and a EE changelog" do + let(:changelog_change) { change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog) } + + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`."]) } + end + end + end + describe '#required_reasons' do subject { changelog.required_reasons } @@ -126,8 +324,8 @@ RSpec.describe Tooling::Danger::Changelog do end end - describe '#found' do - subject { changelog.found } + describe '#present?' do + subject { changelog.present? } context 'added files contain a changelog' do let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) } @@ -138,7 +336,7 @@ RSpec.describe Tooling::Danger::Changelog do context 'added files do not contain a changelog' do let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) } - it { is_expected.to eq(nil) } + it { is_expected.to be_falsy } end end @@ -158,6 +356,22 @@ RSpec.describe Tooling::Danger::Changelog do end end + describe '#changelog_path' do + subject { changelog.changelog_path } + + context 'added files contain a changelog' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) } + + it { is_expected.to eq('foo') } + end + + context 'added files do not contain a changelog' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) } + + it { is_expected.to be_nil } + end + end + describe '#modified_text' do subject { changelog.modified_text } diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index 6bec176b39b..1d2ea0f5ba3 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -256,7 +256,9 @@ RSpec.describe Tooling::Danger::ProjectHelper do subject { project_helper.all_ee_changes } it 'returns all changed files starting with ee/' do - expect(fake_helper).to receive(:all_changed_files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k]) + changes = double + expect(project_helper).to receive(:changes).and_return(changes) + expect(changes).to receive(:files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k]) is_expected.to match_array(%w[ee/wine.rb ee/lib/ido.rb]) end diff --git a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb index fafd5777f93..694ba0f1f4f 100644 --- a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb +++ b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb @@ -72,6 +72,27 @@ RSpec.describe 'layouts/nav/sidebar/_project' do expect(rendered).to have_link('Releases', href: project_releases_path(project), class: 'shortcuts-project-releases') end end + + describe 'Labels' do + let(:page) { Nokogiri::HTML.parse(rendered) } + + it 'has a link to the labels path' do + render + + expect(page.at_css('.shortcuts-project').parent.css('[aria-label="Labels"]')).not_to be_empty + expect(rendered).to have_link('Labels', href: project_labels_path(project)) + end + + context 'when feature flag :sidebar_refactor is disabled' do + it 'does not have the labels menu item' do + stub_feature_flags(sidebar_refactor: false) + + render + + expect(page.at_css('.shortcuts-project').parent.css('[aria-label="Labels"]')).to be_empty + end + end + end end describe 'Learn GitLab' do @@ -181,10 +202,23 @@ RSpec.describe 'layouts/nav/sidebar/_project' do end describe 'Labels' do - it 'has a link to the labels path' do + let(:page) { Nokogiri::HTML.parse(rendered) } + + it 'does not have a link to the labels page' do render - expect(rendered).to have_link('Labels', href: project_labels_path(project)) + expect(page.at_css('.shortcuts-issues').parent.css('[aria-label="Labels"]')).to be_empty + end + + context 'when feature flag :sidebar_refactor is disabled' do + it 'has a link to the labels page' do + stub_feature_flags(sidebar_refactor: false) + + render + + expect(page.at_css('.shortcuts-issues').parent.css('[aria-label="Labels"]')).not_to be_empty + expect(rendered).to have_link('Labels', href: project_labels_path(project)) + end end end @@ -248,21 +282,35 @@ RSpec.describe 'layouts/nav/sidebar/_project' do end describe 'Labels' do - context 'when issues are not enabled' do - it 'has a link to the labels path' do - project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED) + it 'does not show the labels menu' do + project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED) - render + render - expect(rendered).to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels') - end + expect(rendered).not_to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels') end - context 'when issues are enabled' do - it 'does not have a link to the labels path' do - render + context 'when feature flag :sidebar_refactor is disabled' do + before do + stub_feature_flags(sidebar_refactor: false) + end + + context 'when issues are not enabled' do + it 'has a link to the labels path' do + project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED) - expect(rendered).not_to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels') + render + + expect(rendered).to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels') + end + end + + context 'when issues are enabled' do + it 'does not have a link to the labels path' do + render + + expect(rendered).not_to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels') + end end end end diff --git a/spec/workers/web_hook_worker_spec.rb b/spec/workers/web_hook_worker_spec.rb new file mode 100644 index 00000000000..becc7461f2a --- /dev/null +++ b/spec/workers/web_hook_worker_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe WebHookWorker do + include AfterNextHelpers + + let_it_be(:project_hook) { create(:project_hook) } + let_it_be(:data) { { foo: 'bar' } } + let_it_be(:hook_name) { 'push_hooks' } + + describe '#perform' do + it 'delegates to WebHookService' do + expect_next(WebHookService, project_hook, data.with_indifferent_access, hook_name).to receive(:execute) + + subject.perform(project_hook.id, data, hook_name) + end + end +end diff --git a/tooling/danger/changelog.rb b/tooling/danger/changelog.rb index 065c737050e..44b62dcc69e 100644 --- a/tooling/danger/changelog.rb +++ b/tooling/danger/changelog.rb @@ -13,9 +13,10 @@ module Tooling 'meta' ].freeze NO_CHANGELOG_CATEGORIES = %i[docs none].freeze + CHANGELOG_TRAILER_REGEX = /^Changelog:\s*(?<category>.+)$/.freeze CREATE_CHANGELOG_COMMAND = 'bin/changelog -m %<mr_iid>s "%<mr_title>s"' CREATE_EE_CHANGELOG_COMMAND = 'bin/changelog --ee -m %<mr_iid>s "%<mr_title>s"' - CHANGELOG_MODIFIED_URL_TEXT = "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" + CHANGELOG_MODIFIED_URL_TEXT = "**CHANGELOG.md was edited.** Please remove the additions and follow the [changelog guidelines](https://docs.gitlab.com/ee/development/changelog.html).\n\n" CHANGELOG_MISSING_URL_TEXT = "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**:\n\n" OPTIONAL_CHANGELOG_MESSAGE = { @@ -32,6 +33,36 @@ module Tooling If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message. MSG }.freeze + CHANGELOG_NEW_WORKFLOW_MESSAGE = <<~MSG + We are in the process of rolling out a new workflow for adding changelog entries. This new workflow uses Git commit subjects and Git trailers to generate changelogs. This new approach will soon replace the current YAML based approach. + + To ease the transition process, we recommend you start using both the old and new approach in parallel. This is not required at this time, but will make it easier to transition to the new approach in the future. To do so, pick the commit that should go in the changelog and add a `Changelog` trailer to it. For example: + + ``` + This is my commit's subject line + + This is the optional commit body. + + Changelog: added + ``` + + The value of the `Changelog` trailer should be one of the following: added, fixed, changed, deprecated, removed, security, performance, other. + + For more information, take a look at the following resources: + + - `https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/1564` + - https://docs.gitlab.com/ee/api/repositories.html#generate-changelog-data + + If you'd like to see the new approach in action, take a look at the commits in [the Omnibus repository](https://gitlab.com/gitlab-org/omnibus-gitlab/-/commits/master). + MSG + SEE_DOC = "See the [changelog documentation](https://docs.gitlab.com/ee/development/changelog.html)." + SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT + ```suggestion + merge_request: %<mr_iid>s + ``` + + #{SEE_DOC} + SUGGEST_COMMENT REQUIRED_CHANGELOG_REASONS = { db_changes: 'introduces a database migration', @@ -48,6 +79,169 @@ module Tooling MSG }.freeze + CATEGORIES = YAML + .load_file(File.expand_path('../../.gitlab/changelog_config.yml', __dir__)) + .fetch('categories') + .keys + .freeze + + class ChangelogCheckResult + attr_reader :errors, :warnings, :markdowns, :messages + + def initialize(errors: [], warnings: [], markdowns: [], messages: []) + @errors = errors + @warnings = warnings + @markdowns = markdowns + @messages = messages + end + private_class_method :new + + def self.empty + new + end + + def self.error(error) + new(errors: [error]) + end + + def self.warning(warning) + new(warnings: [warning]) + end + + def error(error) + errors << error + end + + def warning(warning) + warnings << warning + end + + def markdown(markdown) + markdowns << markdown + end + + def message(message) + messages << message + end + end + + # rubocop:disable Style/SignalException + def check! + if git.modified_files.include?("CHANGELOG.md") + fail modified_text + end + + if present? + add_danger_messages(check_changelog_yaml) + add_danger_messages(check_changelog_path) + elsif required? + required_texts.each { |_, text| fail(text) } # rubocop:disable Lint/UnreachableLoop + elsif optional? + message optional_text + end + + return unless helper.ci? + + if required? || optional? + checked = 0 + + git.commits.each do |commit| + check_result = check_changelog_trailer(commit) + next if check_result.nil? + + checked += 1 + add_danger_messages(check_result) + end + + if checked == 0 + message CHANGELOG_NEW_WORKFLOW_MESSAGE + end + end + end + # rubocop:enable Style/SignalException + + # rubocop:disable Style/SignalException + def add_danger_messages(check_result) + check_result.errors.each { |error| fail(error) } # rubocop:disable Lint/UnreachableLoop + check_result.warnings.each { |warning| warn(warning) } + check_result.markdowns.each { |markdown_hash| markdown(**markdown_hash) } + check_result.messages.each { |text| message(text) } + end + # rubocop:enable Style/SignalException + + def check_changelog_trailer(commit) + trailer = commit.message.match(CHANGELOG_TRAILER_REGEX) + + return if trailer.nil? || trailer[:category].nil? + + category = trailer[:category] + + return ChangelogCheckResult.empty if CATEGORIES.include?(category) + + ChangelogCheckResult.error("Commit #{commit.sha} uses an invalid changelog category: #{category}") + end + + def check_changelog_yaml + check_result = ChangelogCheckResult.empty + return check_result unless present? + + raw_file = read_file(changelog_path) + yaml = YAML.safe_load(raw_file) + yaml_merge_request = yaml["merge_request"].to_s + + check_result.error("`title` should be set, in #{helper.html_link(changelog_path)}! #{SEE_DOC}") if yaml["title"].nil? + check_result.error("`type` should be set, in #{helper.html_link(changelog_path)}! #{SEE_DOC}") if yaml["type"].nil? + + return check_result if helper.security_mr? || helper.mr_iid.empty? + + check_changelog_yaml_merge_request(raw_file: raw_file, yaml_merge_request: yaml_merge_request, check_result: check_result) + rescue Psych::Exception + # YAML could not be parsed, fail the build. + ChangelogCheckResult.error("#{helper.html_link(changelog_path)} isn't valid YAML! #{SEE_DOC}") + rescue StandardError => e + ChangelogCheckResult.warning("There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}") + end + + def check_changelog_yaml_merge_request(raw_file:, yaml_merge_request:, check_result:) + cherry_pick_against_stable_branch = helper.cherry_pick_mr? && helper.stable_branch? + + if yaml_merge_request.empty? + mr_line = raw_file.lines.find_index { |line| line =~ /merge_request:\s*\n/ } + + if mr_line + check_result.markdown(msg: format(SUGGEST_MR_COMMENT, mr_iid: helper.mr_iid), file: changelog_path, line: mr_line.succ) + else + check_result.message("Consider setting `merge_request` to #{helper.mr_iid} in #{helper.html_link(changelog_path)}. #{SEE_DOC}") + end + elsif yaml_merge_request != helper.mr_iid && !cherry_pick_against_stable_branch + check_result.error("Merge request ID was not set to #{helper.mr_iid}! #{SEE_DOC}") + end + + check_result + end + + def check_changelog_path + check_result = ChangelogCheckResult.empty + return check_result unless present? + + ee_changes = project_helper.all_ee_changes.dup + ee_changes.delete(changelog_path) + + if ee_changes.any? && !ee_changelog? && !required? + check_result.warning("This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`.") + end + + if ee_changes.empty? && ee_changelog? + check_result.warning("This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`.") + end + + if ee_changes.any? && ee_changelog? && required_reasons.include?(:db_changes) + check_result.warning("This MR has a Changelog file inside `ee/`, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog placement to be outside of `ee/`. Consider moving the Changelog file outside `ee/`.") + end + + check_result + end + def required_reasons [].tap do |reasons| reasons << :db_changes if project_helper.changes.added.has_category?(:migration) @@ -60,15 +254,19 @@ module Tooling end def optional? - categories_need_changelog? && without_no_changelog_label? + categories_need_changelog? && mr_without_no_changelog_label? end - def found - @found ||= project_helper.changes.added.by_category(:changelog).files.first + def present? + !!changelog_path end def ee_changelog? - found.start_with?('ee/') + changelog_path.start_with?('ee/') + end + + def changelog_path + @changelog_path ||= project_helper.changes.added.by_category(:changelog).files.first end def modified_text @@ -91,6 +289,10 @@ module Tooling private + def read_file(path) + File.read(path) + end + def sanitized_mr_title Gitlab::Dangerfiles::TitleLinting.sanitize_mr_title(helper.mr_title) end @@ -99,7 +301,7 @@ module Tooling (project_helper.changes.categories - NO_CHANGELOG_CATEGORIES).any? end - def without_no_changelog_label? + def mr_without_no_changelog_label? (helper.mr_labels & NO_CHANGELOG_LABELS).empty? end end diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb index b60a3aa1adc..c6aaf82ef35 100644 --- a/tooling/danger/project_helper.rb +++ b/tooling/danger/project_helper.rb @@ -168,7 +168,7 @@ module Tooling end def all_ee_changes - helper.all_changed_files.grep(%r{\Aee/}) + changes.files.grep(%r{\Aee/}) end def project_name |
