From eac94e5cd6a59aad41ff1f86ed7cc892898d8516 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 1 Nov 2022 15:11:22 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../job/sidebar/legacy_sidebar_header.vue | 9 +- app/assets/javascripts/jobs/constants.js | 1 + .../components/widget/widget_content_row.vue | 25 +-- .../merge_requests/diffs_controller.rb | 8 +- app/helpers/diff_helper.rb | 1 - .../integrations/base_slack_notification.rb | 62 ++++++++ app/models/integrations/slack.rb | 60 +------ app/models/merge_request.rb | 2 +- app/serializers/diff_file_entity.rb | 21 +-- app/serializers/diffs_entity.rb | 2 +- app/serializers/diffs_metadata_entity.rb | 2 +- app/serializers/paginated_diff_entity.rb | 2 +- .../mergeability_check_service.rb | 3 +- app/views/award_emoji/_awards_block.html.haml | 14 +- .../projects/_import_project_pane.html.haml | 53 ++----- .../namespaces/root_statistics_worker.rb | 2 +- config/feature_categories.yml | 1 - .../root_statistics_worker_read_replica.yml | 8 + db/structure.sql | 20 ++- doc/ci/yaml/index.md | 7 + locale/gitlab.pot | 29 +++- .../merge_requests/diffs_controller_spec.rb | 12 +- .../pipelines/legacy_pipeline_spec.rb | 7 +- .../projects/pipelines/pipeline_spec.rb | 7 +- .../job/legacy_sidebar_header_spec.js | 20 ++- spec/frontend/jobs/mock_data.js | 23 +++ .../widget/widget_content_row_spec.js | 2 + spec/helpers/diff_helper_spec.rb | 10 -- .../analytics/cycle_analytics/median_spec.rb | 2 +- .../Jobs/code_quality_gitlab_ci_yaml_spec.rb | 11 +- .../Jobs/sast_iac_gitlab_ci_yaml_spec.rb | 10 +- .../sast_iac_latest_gitlab_ci_yaml_spec.rb | 10 +- .../Jobs/test_gitlab_ci_yaml_spec.rb | 11 +- spec/lib/gitlab/ci/templates/npm_spec.rb | 5 +- .../templates/themekit_gitlab_ci_yaml_spec.rb | 7 +- spec/lib/gitlab/sidekiq_migrate_jobs_spec.rb | 76 +++++++++ spec/models/integrations/slack_spec.rb | 140 +--------------- spec/models/merge_request_spec.rb | 12 +- .../context_commit_diffs_spec.rb | 2 +- .../projects/merge_requests/diffs_spec.rb | 4 +- spec/serializers/diff_file_entity_spec.rb | 44 +---- spec/serializers/diffs_entity_spec.rb | 73 +++------ .../serializers/diffs_metadata_entity_spec.rb | 53 +++---- .../serializers/paginated_diff_entity_spec.rb | 76 ++++----- .../mergeability_check_service_spec.rb | 30 ---- ...base_slack_notification_shared_examples.rb | 150 ++++++++++++++++++ .../namespaces/root_statistics_worker_spec.rb | 5 + 47 files changed, 575 insertions(+), 559 deletions(-) create mode 100644 app/models/integrations/base_slack_notification.rb create mode 100644 config/feature_flags/development/root_statistics_worker_read_replica.yml create mode 100644 spec/support/shared_examples/models/concerns/integrations/base_slack_notification_shared_examples.rb diff --git a/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue b/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue index 263b2d141c9..64b497c3550 100644 --- a/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue +++ b/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue @@ -35,6 +35,11 @@ export default { retryButtonCategory() { return this.job.status && this.job.recoverable ? 'primary' : 'secondary'; }, + buttonTitle() { + return this.job.status && this.job.status.text === 'passed' + ? this.$options.i18n.runAgainJobButtonLabel + : this.$options.i18n.retryJobButtonLabel; + }, }, methods: { ...mapActions(['toggleSidebar']), @@ -66,8 +71,8 @@ export default { -
- -
- +
+
+ +
+ +
+ +
+
- +
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index f9a11ddb1db..c88dbc70ed5 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -44,7 +44,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic diff_view: diff_view, merge_ref_head_diff: render_merge_ref_head_diff?, pagination_data: diffs.pagination_data, - allow_tree_conflicts: display_merge_conflicts_in_diff? + merge_conflicts_in_diff: display_merge_conflicts_in_diff? } # NOTE: Any variables that would affect the resulting json needs to be added to the cache_context to avoid stale cache issues. @@ -57,7 +57,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic params[:page], params[:per_page], options[:merge_ref_head_diff], - options[:allow_tree_conflicts] + options[:merge_conflicts_in_diff] ] if Feature.enabled?(:check_etags_diffs_batch_before_write_cache, merge_request.project) && !stale?(etag: [cache_context + diff_options_hash.fetch(:paths, []), diffs]) @@ -81,7 +81,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = additional_attributes.merge( only_context_commits: show_only_context_commits?, merge_ref_head_diff: render_merge_ref_head_diff?, - allow_tree_conflicts: display_merge_conflicts_in_diff? + merge_conflicts_in_diff: display_merge_conflicts_in_diff? ) render json: DiffsMetadataSerializer.new(project: @merge_request.project, current_user: current_user) @@ -110,7 +110,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = additional_attributes.merge( diff_view: "inline", merge_ref_head_diff: render_merge_ref_head_diff?, - allow_tree_conflicts: display_merge_conflicts_in_diff? + merge_conflicts_in_diff: display_merge_conflicts_in_diff? ) options[:context_commits] = @merge_request.recent_context_commits diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index b8329dada81..aca811ca5b6 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -227,7 +227,6 @@ module DiffHelper end def conflicts(allow_tree_conflicts: false) - return unless options[:merge_ref_head_diff] return unless merge_request.cannot_be_merged? conflicts_service = MergeRequests::Conflicts::ListService.new(merge_request, allow_tree_conflicts: allow_tree_conflicts) # rubocop:disable CodeReuse/ServiceClass diff --git a/app/models/integrations/base_slack_notification.rb b/app/models/integrations/base_slack_notification.rb new file mode 100644 index 00000000000..cb785afdcfe --- /dev/null +++ b/app/models/integrations/base_slack_notification.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Integrations + class BaseSlackNotification < BaseChatNotification + SUPPORTED_EVENTS_FOR_USAGE_LOG = %w[ + push issue confidential_issue merge_request note confidential_note tag_push wiki_page deployment + ].freeze + + prop_accessor EVENT_CHANNEL['alert'] + + override :default_channel_placeholder + def default_channel_placeholder + _('#general, #development') + end + + override :get_message + def get_message(object_kind, data) + return Integrations::ChatMessage::AlertMessage.new(data) if object_kind == 'alert' + + super + end + + override :supported_events + def supported_events + additional = ['alert'] + + super + additional + end + + override :configurable_channels? + def configurable_channels? + true + end + + override :log_usage + def log_usage(event, user_id) + return unless user_id + + return unless SUPPORTED_EVENTS_FOR_USAGE_LOG.include?(event) + + key = "i_ecosystem_slack_service_#{event}_notification" + + Gitlab::UsageDataCounters::HLLRedisCounter.track_event(key, values: user_id) + + return unless Feature.enabled?(:route_hll_to_snowplow_phase2) + + optional_arguments = { + project: project, + namespace: group || project&.namespace + }.compact + + Gitlab::Tracking.event( + self.class.name, + Integration::SNOWPLOW_EVENT_ACTION, + label: Integration::SNOWPLOW_EVENT_LABEL, + property: key, + user: User.find(user_id), + **optional_arguments + ) + end + end +end diff --git a/app/models/integrations/slack.rb b/app/models/integrations/slack.rb index 834013e18f1..ffca0d40f69 100644 --- a/app/models/integrations/slack.rb +++ b/app/models/integrations/slack.rb @@ -1,17 +1,9 @@ # frozen_string_literal: true module Integrations - class Slack < BaseChatNotification + class Slack < BaseSlackNotification include SlackMattermostNotifier - SUPPORTED_EVENTS_FOR_USAGE_LOG = %w[ - push issue confidential_issue merge_request note confidential_note - tag_push wiki_page deployment - ].freeze - SNOWPLOW_EVENT_CATEGORY = self.name - - prop_accessor EVENT_CHANNEL['alert'] - def title 'Slack notifications' end @@ -24,57 +16,9 @@ module Integrations 'slack' end - def default_channel_placeholder - _('#general, #development') - end - + override :webhook_placeholder def webhook_placeholder 'https://hooks.slack.com/services/…' end - - def supported_events - additional = [] - additional << 'alert' - - super + additional - end - - def get_message(object_kind, data) - return Integrations::ChatMessage::AlertMessage.new(data) if object_kind == 'alert' - - super - end - - override :log_usage - def log_usage(event, user_id) - return unless user_id - - return unless SUPPORTED_EVENTS_FOR_USAGE_LOG.include?(event) - - key = "i_ecosystem_slack_service_#{event}_notification" - - Gitlab::UsageDataCounters::HLLRedisCounter.track_event(key, values: user_id) - - return unless Feature.enabled?(:route_hll_to_snowplow_phase2) - - optional_arguments = { - project: project, - namespace: group || project&.namespace - }.compact - - Gitlab::Tracking.event( - SNOWPLOW_EVENT_CATEGORY, - Integration::SNOWPLOW_EVENT_ACTION, - label: Integration::SNOWPLOW_EVENT_LABEL, - property: key, - user: User.find(user_id), - **optional_arguments - ) - end - - override :configurable_channels? - def configurable_channels? - true - end end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 220dd104335..f7b57c5a442 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1134,7 +1134,7 @@ class MergeRequest < ApplicationRecord # rubocop: enable CodeReuse/ServiceClass def diffable_merge_ref? - open? && merge_head_diff.present? && (Feature.enabled?(:display_merge_conflicts_in_diff, project) || can_be_merged?) + open? && merge_head_diff.present? && can_be_merged? end # Returns boolean indicating the merge_status should be rechecked in order to diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb index 9f8628fe849..aa43b9861d3 100644 --- a/app/serializers/diff_file_entity.rb +++ b/app/serializers/diff_file_entity.rb @@ -55,17 +55,9 @@ class DiffFileEntity < DiffFileBaseEntity end # Used for inline diffs - expose :highlighted_diff_lines, using: DiffLineEntity, if: -> (diff_file, options) { inline_diff_view?(options) && diff_file.text? } do |diff_file| - highlighted_diff_lines_for(diff_file, options) - end + expose :diff_lines_for_serializer, as: :highlighted_diff_lines, using: DiffLineEntity, if: -> (diff_file, options) { inline_diff_view?(options) && diff_file.text? } - expose :is_fully_expanded do |diff_file| - if conflict_file(options, diff_file) - false - else - diff_file.fully_expanded? - end - end + expose :fully_expanded?, as: :is_fully_expanded # Used for parallel diffs expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, options) { parallel_diff_view?(options) && diff_file.text? } @@ -88,15 +80,6 @@ class DiffFileEntity < DiffFileBaseEntity # If nothing is present, inline will be the default. options.fetch(:diff_view, :inline).to_sym end - - def highlighted_diff_lines_for(diff_file, options) - file = conflict_file(options, diff_file) || diff_file - - file.diff_lines_for_serializer - rescue Gitlab::Git::Conflict::Parser::UnmergeableFile - # Fallback to diff_file as it means that conflict lines can't be parsed due to limit - diff_file.diff_lines_for_serializer - end end DiffFileEntity.prepend_mod diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb index c818fcd6215..759d1e0f10a 100644 --- a/app/serializers/diffs_entity.rb +++ b/app/serializers/diffs_entity.rb @@ -74,7 +74,7 @@ class DiffsEntity < Grape::Entity options.merge( submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs), - conflicts: conflicts(allow_tree_conflicts: options[:allow_tree_conflicts]) + conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff]) ) ) end diff --git a/app/serializers/diffs_metadata_entity.rb b/app/serializers/diffs_metadata_entity.rb index 8c226130f6e..ace5105dda5 100644 --- a/app/serializers/diffs_metadata_entity.rb +++ b/app/serializers/diffs_metadata_entity.rb @@ -6,7 +6,7 @@ class DiffsMetadataEntity < DiffsEntity DiffFileMetadataEntity.represent( diffs.raw_diff_files(sorted: true), options.merge( - conflicts: conflicts(allow_tree_conflicts: options[:allow_tree_conflicts]) + conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff]) ) ) end diff --git a/app/serializers/paginated_diff_entity.rb b/app/serializers/paginated_diff_entity.rb index c656cff9dd7..b79a0937659 100644 --- a/app/serializers/paginated_diff_entity.rb +++ b/app/serializers/paginated_diff_entity.rb @@ -17,7 +17,7 @@ class PaginatedDiffEntity < Grape::Entity options.merge( submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs), - conflicts: conflicts(allow_tree_conflicts: options[:allow_tree_conflicts]) + conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff]) ) ) end diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index 1ce44f465cd..2a3f417a33b 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -156,8 +156,7 @@ module MergeRequests end def merge_to_ref - params = { allow_conflicts: Feature.enabled?(:display_merge_conflicts_in_diff, project) } - result = MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author, params: params).execute(merge_request) + result = MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author, params: {}).execute(merge_request) result[:status] == :success end diff --git a/app/views/award_emoji/_awards_block.html.haml b/app/views/award_emoji/_awards_block.html.haml index 5062599c261..6bdf1f7aae1 100644 --- a/app/views/award_emoji/_awards_block.html.haml +++ b/app/views/award_emoji/_awards_block.html.haml @@ -8,19 +8,15 @@ - grouped_emojis = awardable.grouped_awards(with_thumbs: inline) .awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.empty?), data: { award_url: toggle_award_url(awardable) } } - awards_sort(grouped_emojis).each do |emoji, awards| - %button.gl-button.btn.btn-default.award-control.js-emoji-btn.has-tooltip{ type: "button", - class: [award_state_class(awardable, awards, current_user)], - data: { title: award_user_list(awards, current_user) } } + = render Pajamas::ButtonComponent.new(button_options: { class: "#{award_state_class(awardable, awards, current_user)} award-control js-emoji-btn", title: award_user_list(awards, current_user), data: { toggle: 'tooltip', container: 'body' } }) do = emoji_icon(emoji) %span.award-control-text.js-counter = awards.count - if can?(current_user, :award_emoji, awardable) .award-menu-holder.js-award-holder - %button.gl-button.btn.btn-default.award-control.has-tooltip.js-add-award{ type: 'button', - 'aria-label': _('Add reaction'), - data: { title: _('Add reaction') } } - %span{ class: "award-control-icon award-control-icon-neutral gl-icon" }= sprite_icon('slight-smile') - %span{ class: "award-control-icon award-control-icon-positive gl-icon" }= sprite_icon('smiley') - %span{ class: "award-control-icon award-control-icon-super-positive gl-icon" }= sprite_icon('smile') + = render Pajamas::ButtonComponent.new(button_text_classes: 'gl-display-flex', button_options: { title: _('Add reaction'), class: 'js-add-award award-control has-tooltip', 'aria-label': _('Add reaction') }) do + = sprite_icon('slight-smile', css_class: 'award-control-icon-neutral gl-icon gl-mr-0!') + = sprite_icon('smiley', css_class: 'award-control-icon-positive gl-icon') + = sprite_icon('smile', css_class: 'award-control-icon-super-positive gl-icon') = yield diff --git a/app/views/projects/_import_project_pane.html.haml b/app/views/projects/_import_project_pane.html.haml index 42cdc1d6989..afc7fb3d8b6 100644 --- a/app/views/projects/_import_project_pane.html.haml +++ b/app/views/projects/_import_project_pane.html.haml @@ -10,45 +10,32 @@ .import-buttons - if gitlab_project_import_enabled? .import_gitlab_project.has-tooltip{ data: { container: 'body', qa_selector: 'gitlab_import_button' } } - = link_to '#', class: 'gl-button btn-default btn btn_import_gitlab_project js-import-project-btn', data: { href: new_import_gitlab_project_path, platform: 'gitlab_export', **tracking_attrs_data(track_label, 'click_button', 'gitlab_export') } do - .gl-button-icon - = sprite_icon('tanuki') - = _("GitLab export") + = render Pajamas::ButtonComponent.new(href: '#', icon: 'tanuki', button_options: { class: 'btn_import_gitlab_project js-import-project-btn', data: { href: new_import_gitlab_project_path, platform: 'gitlab_export', **tracking_attrs_data(track_label, 'click_button', 'gitlab_export') } }) do + = _('GitLab export') + + - if gitlab_import_enabled? + %div + = render Pajamas::ButtonComponent.new(href: status_import_gitlab_path(namespace_id: namespace_id), icon: 'tanuki', button_options: { class: "import_gitlab js-import-project-btn #{'js-how-to-import-link' unless gitlab_import_configured?}", data: { modal_title: _("Import projects from GitLab.com"), modal_message: import_from_gitlab_message, platform: 'gitlab_com', **tracking_attrs_data(track_label, 'click_button', 'gitlab_com') } }) do + GitLab.com - if github_import_enabled? %div - = link_to new_import_github_path(namespace_id: namespace_id), class: 'gl-button btn-default btn js-import-github js-import-project-btn', data: { platform: 'github', **tracking_attrs_data(track_label, 'click_button', 'github') } do - .gl-button-icon - = sprite_icon('github') + = render Pajamas::ButtonComponent.new(href: new_import_github_path(namespace_id: namespace_id), icon: 'github', button_options: { class: 'js-import-github js-import-project-btn', data: { platform: 'github', **tracking_attrs_data(track_label, 'click_button', 'github') } }) do GitHub - if bitbucket_import_enabled? %div - = link_to status_import_bitbucket_path(namespace_id: namespace_id), class: "gl-button btn-default btn import_bitbucket js-import-project-btn #{'js-how-to-import-link' unless bitbucket_import_configured?}", - data: { modal_title: _("Import projects from Bitbucket"), modal_message: import_from_bitbucket_message, platform: 'bitbucket_cloud', **tracking_attrs_data(track_label, 'click_button', 'bitbucket_cloud') } do - .gl-button-icon - = sprite_icon('bitbucket') + = render Pajamas::ButtonComponent.new(href: status_import_bitbucket_path(namespace_id: namespace_id), icon: 'bitbucket', button_options: { class: "import_bitbucket js-import-project-btn #{'js-how-to-import-link' unless bitbucket_import_configured?}", data: { modal_title: _("Import projects from Bitbucket"), modal_message: import_from_bitbucket_message, platform: 'bitbucket_cloud', **tracking_attrs_data(track_label, 'click_button', 'bitbucket_cloud') } }) do Bitbucket Cloud + - if bitbucket_server_import_enabled? %div - = link_to status_import_bitbucket_server_path(namespace_id: namespace_id), class: "gl-button btn-default btn import_bitbucket js-import-project-btn", data: { platform: 'bitbucket_server', **tracking_attrs_data(track_label, 'click_button', 'bitbucket_server') } do - .gl-button-icon - = sprite_icon('bitbucket') + = render Pajamas::ButtonComponent.new(href: status_import_bitbucket_server_path(namespace_id: namespace_id), icon: 'bitbucket', button_options: { class: 'import_bitbucket js-import-project-btn', data: { platform: 'bitbucket_server', **tracking_attrs_data(track_label, 'click_button', 'bitbucket_server') } }) do Bitbucket Server - %div - - if gitlab_import_enabled? - %div - = link_to status_import_gitlab_path(namespace_id: namespace_id), class: "gl-button btn-default btn import_gitlab js-import-project-btn #{'js-how-to-import-link' unless gitlab_import_configured?}", - data: { modal_title: _("Import projects from GitLab.com"), modal_message: import_from_gitlab_message, platform: 'gitlab_com', **tracking_attrs_data(track_label, 'click_button', 'gitlab_com') } do - .gl-button-icon - = sprite_icon('tanuki') - = _("GitLab.com") - if fogbugz_import_enabled? %div - = link_to new_import_fogbugz_path(namespace_id: namespace_id), class: 'gl-button btn-default btn import_fogbugz js-import-project-btn', data: { platform: 'fogbugz', **tracking_attrs_data(track_label, 'click_button', 'fogbugz') } do - .gl-button-icon - = sprite_icon('bug') + = render Pajamas::ButtonComponent.new(href: new_import_fogbugz_path(namespace_id: namespace_id), icon: 'bug', button_options: { class: 'import_fogbugz js-import-project-btn', data: { platform: 'fogbugz', **tracking_attrs_data(track_label, 'click_button', 'fogbugz') } }) do FogBugz - if gitea_import_enabled? @@ -60,24 +47,18 @@ - if git_import_enabled? %div - %button.gl-button.btn-default.btn.btn-svg.js-toggle-button.js-import-git-toggle-button.js-import-project-btn{ type: "button", data: { platform: 'repo_url', toggle_open_class: 'active', **tracking_attrs_data(track_label, 'click_button', 'repo_url') } } - .gl-button-icon - = sprite_icon('link', css_class: 'gl-icon') + = render Pajamas::ButtonComponent.new(icon: 'link', button_options: { class: 'js-toggle-button js-import-git-toggle-button js-import-project-btn', data: { platform: 'repo_url', toggle_open_class: 'active', **tracking_attrs_data(track_label, 'click_button', 'repo_url') } }) do = _('Repository by URL') - if manifest_import_enabled? %div - = link_to new_import_manifest_path(namespace_id: namespace_id), class: 'gl-button btn-default btn import_manifest js-import-project-btn', data: { platform: 'manifest_file', **tracking_attrs_data(track_label, 'click_button', 'manifest_file') } do - .gl-button-icon - = sprite_icon('doc-text') - Manifest file + = render Pajamas::ButtonComponent.new(href: new_import_manifest_path(namespace_id: namespace_id), icon: 'doc-text', button_options: { class: 'import_manifest js-import-project-btn', data: { platform: 'manifest_file', **tracking_attrs_data(track_label, 'click_button', 'manifest_file') } }) do + = _('Manifest file') - if phabricator_import_enabled? %div - = link_to new_import_phabricator_path(namespace_id: namespace_id), class: 'gl-button btn-default btn import_phabricator js-import-project-btn', data: { platform: 'phabricator', track_label: "#{track_label}", track_action: "click_button", track_property: "phabricator" } do - .gl-button-icon - = custom_icon('issues') - = _("Phabricator Tasks") + = render Pajamas::ButtonComponent.new(href: new_import_phabricator_path(namespace_id: namespace_id), icon: 'issues', button_options: { class: 'import_phabricator js-import-project-btn', data: { platform: 'phabricator', track_label: "#{track_label}", track_action: "click_button", track_property: "phabricator" } }) do + = _('Phabricator tasks') .js-toggle-content.toggle-import-form{ class: ('hide' if active_tab != 'import') } diff --git a/app/workers/namespaces/root_statistics_worker.rb b/app/workers/namespaces/root_statistics_worker.rb index 157a779dda6..e3aa8a1f779 100644 --- a/app/workers/namespaces/root_statistics_worker.rb +++ b/app/workers/namespaces/root_statistics_worker.rb @@ -4,7 +4,7 @@ module Namespaces class RootStatisticsWorker include ApplicationWorker - data_consistency :always + data_consistency :sticky, feature_flag: :root_statistics_worker_read_replica sidekiq_options retry: 3 diff --git a/config/feature_categories.yml b/config/feature_categories.yml index b30525da291..94a50dc416e 100644 --- a/config/feature_categories.yml +++ b/config/feature_categories.yml @@ -100,7 +100,6 @@ - planning_analytics - pods - portfolio_management -- privacy_control_center - product_analytics - projects - pubsec_services diff --git a/config/feature_flags/development/root_statistics_worker_read_replica.yml b/config/feature_flags/development/root_statistics_worker_read_replica.yml new file mode 100644 index 00000000000..516bead1ee7 --- /dev/null +++ b/config/feature_flags/development/root_statistics_worker_read_replica.yml @@ -0,0 +1,8 @@ +--- +name: root_statistics_worker_read_replica +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102516 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/379678 +milestone: '15.6' +type: development +group: group::utilization +default_enabled: false diff --git a/db/structure.sql b/db/structure.sql index 8aef35f9cb8..e1955b44f97 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11434,6 +11434,7 @@ CREATE TABLE application_settings ( inactive_projects_min_size_mb integer DEFAULT 0 NOT NULL, inactive_projects_send_warning_email_after_months integer DEFAULT 1 NOT NULL, delayed_group_deletion boolean DEFAULT true NOT NULL, + maven_package_requests_forwarding boolean DEFAULT true NOT NULL, arkose_labs_namespace text DEFAULT 'client'::text NOT NULL, max_export_size integer DEFAULT 0, encrypted_slack_app_signing_secret bytea, @@ -11472,18 +11473,17 @@ CREATE TABLE application_settings ( cube_api_base_url text, encrypted_cube_api_key bytea, encrypted_cube_api_key_iv bytea, - maven_package_requests_forwarding boolean DEFAULT true NOT NULL, - dashboard_limit_enabled boolean DEFAULT false NOT NULL, - dashboard_limit integer DEFAULT 0 NOT NULL, - dashboard_notification_limit integer DEFAULT 0 NOT NULL, - dashboard_enforcement_limit integer DEFAULT 0 NOT NULL, - dashboard_limit_new_namespace_creation_enforcement_date date, jitsu_host text, jitsu_project_xid text, clickhouse_connection_string text, jitsu_administrator_email text, encrypted_jitsu_administrator_password bytea, encrypted_jitsu_administrator_password_iv bytea, + dashboard_limit_enabled boolean DEFAULT false NOT NULL, + dashboard_limit integer DEFAULT 0 NOT NULL, + dashboard_notification_limit integer DEFAULT 0 NOT NULL, + dashboard_enforcement_limit integer DEFAULT 0 NOT NULL, + dashboard_limit_new_namespace_creation_enforcement_date date, can_create_group boolean DEFAULT true NOT NULL, lock_maven_package_requests_forwarding boolean DEFAULT false NOT NULL, lock_pypi_package_requests_forwarding boolean DEFAULT false NOT NULL, @@ -20192,8 +20192,8 @@ CREATE TABLE project_settings ( selective_code_owner_removals boolean DEFAULT false NOT NULL, issue_branch_template text, show_diff_preview_in_email boolean DEFAULT true NOT NULL, - suggested_reviewers_enabled boolean DEFAULT false NOT NULL, jitsu_key text, + suggested_reviewers_enabled boolean DEFAULT false NOT NULL, only_allow_merge_if_all_status_checks_passed boolean DEFAULT false NOT NULL, mirror_branch_regex text, CONSTRAINT check_2981f15877 CHECK ((char_length(jitsu_key) <= 100)), @@ -28246,6 +28246,10 @@ CREATE UNIQUE INDEX p_ci_builds_metadata_build_id_partition_id_idx ON ONLY p_ci_ CREATE UNIQUE INDEX index_ci_builds_metadata_on_build_id_partition_id_unique ON ci_builds_metadata USING btree (build_id, partition_id); +CREATE UNIQUE INDEX p_ci_builds_metadata_id_partition_id_idx ON ONLY p_ci_builds_metadata USING btree (id, partition_id); + +CREATE UNIQUE INDEX index_ci_builds_metadata_on_id_partition_id_unique ON ci_builds_metadata USING btree (id, partition_id); + CREATE INDEX p_ci_builds_metadata_project_id_idx ON ONLY p_ci_builds_metadata USING btree (project_id); CREATE INDEX index_ci_builds_metadata_on_project_id ON ci_builds_metadata USING btree (project_id); @@ -32484,6 +32488,8 @@ ALTER INDEX p_ci_builds_metadata_build_id_id_idx ATTACH PARTITION index_ci_build ALTER INDEX p_ci_builds_metadata_build_id_partition_id_idx ATTACH PARTITION index_ci_builds_metadata_on_build_id_partition_id_unique; +ALTER INDEX p_ci_builds_metadata_id_partition_id_idx ATTACH PARTITION index_ci_builds_metadata_on_id_partition_id_unique; + ALTER INDEX p_ci_builds_metadata_project_id_idx ATTACH PARTITION index_ci_builds_metadata_on_project_id; CREATE TRIGGER chat_names_loose_fk_trigger AFTER DELETE ON chat_names REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index a32ba9751c4..7c446635096 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -2835,6 +2835,13 @@ deploystacks: [vultr, data] deploystacks: [vultr, processing] ``` +**Additional details**: + +- `parallel:matrix` jobs add the variable values to the job names to differentiate + the jobs from each other, but [large values can cause names to exceed limits](https://gitlab.com/gitlab-org/gitlab/-/issues/362262): + - Job names must be [255 characters or fewer](../jobs/index.md#job-name-limitations). + - When using [`needs`](#needs), job names must be 128 characters or fewer. + **Related topics**: - [Run a one-dimensional matrix of parallel jobs](../jobs/job_control.md#run-a-one-dimensional-matrix-of-parallel-jobs). diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4d73fcd48fa..ef67a5be10c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18236,9 +18236,6 @@ msgstr "" msgid "GitLab will create a branch in your fork and start a merge request." msgstr "" -msgid "GitLab.com" -msgstr "" - msgid "GitLab.com (SaaS)" msgstr "" @@ -23390,6 +23387,9 @@ msgstr "" msgid "Job|Retry" msgstr "" +msgid "Job|Run again" +msgstr "" + msgid "Job|Running" msgstr "" @@ -24744,6 +24744,9 @@ msgstr "" msgid "Manifest" msgstr "" +msgid "Manifest file" +msgstr "" + msgid "Manifest file import" msgstr "" @@ -29450,7 +29453,7 @@ msgstr "" msgid "Phabricator Server URL" msgstr "" -msgid "Phabricator Tasks" +msgid "Phabricator tasks" msgstr "" msgid "Phone" @@ -47661,12 +47664,27 @@ msgstr "" msgid "ciReport|Dependency scanning" msgstr "" +msgid "ciReport|Detects known vulnerabilities in your source code's dependencies." +msgstr "" + +msgid "ciReport|Detects known vulnerabilities in your source code." +msgstr "" + +msgid "ciReport|Detects known vulnerabilities in your web application." +msgstr "" + +msgid "ciReport|Detects secrets and credentials vulnerabilities in your source code." +msgstr "" + msgid "ciReport|Download patch to resolve" msgstr "" msgid "ciReport|Download the patch to apply it manually" msgstr "" +msgid "ciReport|Dynamic Application Security Testing (DAST)" +msgstr "" + msgid "ciReport|Dynamic Application Security Testing (DAST) detects known vulnerabilities in your web application." msgstr "" @@ -47786,6 +47804,9 @@ msgstr "" msgid "ciReport|Solution" msgstr "" +msgid "ciReport|Static Application Security Testing (SAST)" +msgstr "" + msgid "ciReport|Static Application Security Testing (SAST) detects known vulnerabilities in your source code." msgstr "" diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 367781c0e76..613d82efd06 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -213,7 +213,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, merge_ref_head_diff: false } end @@ -281,7 +281,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, merge_ref_head_diff: nil } end @@ -303,7 +303,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: merge_request.diff_head_commit, latest_diff: nil, only_context_commits: false, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, merge_ref_head_diff: nil } end @@ -329,7 +329,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - allow_tree_conflicts: false, + merge_conflicts_in_diff: false, merge_ref_head_diff: nil } end @@ -488,7 +488,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -616,7 +616,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'serializes diffs with expected arguments' do let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(total_pages: 20).merge(allow_tree_conflicts: false) } + let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) } end it_behaves_like 'successful request' diff --git a/spec/features/projects/pipelines/legacy_pipeline_spec.rb b/spec/features/projects/pipelines/legacy_pipeline_spec.rb index d93c951791d..c4fc194f0cd 100644 --- a/spec/features/projects/pipelines/legacy_pipeline_spec.rb +++ b/spec/features/projects/pipelines/legacy_pipeline_spec.rb @@ -726,12 +726,7 @@ RSpec.describe 'Pipeline', :js do before do schedule.owner.block! - - begin - PipelineScheduleWorker.new.perform - rescue Ci::CreatePipelineService::CreateError - # Do nothing, assert view code after the Pipeline failed to create. - end + PipelineScheduleWorker.new.perform end it 'displays the PipelineSchedule in an inactive state' do diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 9a99f16b669..66916aea8d7 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -851,12 +851,7 @@ RSpec.describe 'Pipeline', :js do before do schedule.owner.block! - - begin - PipelineScheduleWorker.new.perform - rescue Ci::CreatePipelineService::CreateError - # Do nothing, assert view code after the Pipeline failed to create. - end + PipelineScheduleWorker.new.perform end it 'displays the PipelineSchedule in an inactive state' do diff --git a/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js b/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js index cb32ca9d3dc..95eb10118ee 100644 --- a/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js +++ b/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js @@ -3,7 +3,7 @@ import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import JobRetryButton from '~/jobs/components/job/sidebar/job_sidebar_retry_button.vue'; import LegacySidebarHeader from '~/jobs/components/job/sidebar/legacy_sidebar_header.vue'; import createStore from '~/jobs/store'; -import job from '../../mock_data'; +import job, { failedJobStatus } from '../../mock_data'; describe('Legacy Sidebar Header', () => { let store; @@ -67,6 +67,12 @@ describe('Legacy Sidebar Header', () => { it('should render the retry button', () => { expect(findRetryButton().props('href')).toBe(job.retry_path); }); + + it('should have a different label when the job status is passed', () => { + expect(findRetryButton().attributes('title')).toBe( + LegacySidebarHeader.i18n.runAgainJobButtonLabel, + ); + }); }); describe('when there is no retry path', () => { @@ -88,4 +94,16 @@ describe('Legacy Sidebar Header', () => { expect(findCancelButton().attributes('href')).toBe(job.cancel_path); }); }); + + describe('when the job is failed', () => { + describe('retry button', () => { + it('should have a different label when the job status is failed', () => { + createWrapper({ job: { ...job, status: failedJobStatus } }); + + expect(findRetryButton().attributes('title')).toBe( + LegacySidebarHeader.i18n.retryJobButtonLabel, + ); + }); + }); + }); }); diff --git a/spec/frontend/jobs/mock_data.js b/spec/frontend/jobs/mock_data.js index 1dfd43ad21a..a7fe6d5a626 100644 --- a/spec/frontend/jobs/mock_data.js +++ b/spec/frontend/jobs/mock_data.js @@ -1086,6 +1086,29 @@ export default { has_trace: true, }; +export const failedJobStatus = { + icon: 'status_warning', + text: 'failed', + label: 'failed (allowed to fail)', + group: 'failed-with-warnings', + tooltip: 'failed - (unknown failure) (allowed to fail)', + has_details: true, + details_path: '/gitlab-org/gitlab-shell/-/jobs/454', + illustration: { + image: 'illustrations/skipped-job_empty.svg', + size: 'svg-430', + title: 'This job does not have a trace.', + }, + favicon: + '/assets/ci_favicons/favicon_status_failed-41304d7f7e3828808b0c26771f0309e55296819a9beea3ea9fbf6689d9857c12.png', + action: { + icon: 'retry', + title: 'Retry', + path: '/gitlab-org/gitlab-shell/-/jobs/454/retry', + method: 'post', + }, +}; + export const jobsInStage = { name: 'build', title: 'build: running', diff --git a/spec/frontend/vue_merge_request_widget/components/widget/widget_content_row_spec.js b/spec/frontend/vue_merge_request_widget/components/widget/widget_content_row_spec.js index 9eddd091ad0..f885db86ea1 100644 --- a/spec/frontend/vue_merge_request_widget/components/widget/widget_content_row_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/widget/widget_content_row_spec.js @@ -36,12 +36,14 @@ describe('~/vue_merge_request_widget/components/widget/widget_content_row.vue', }, slots: { header: 'this is a header', + 'header-actions': 'this is a header action', body: 'this is a body', }, }); expect(wrapper.findByText('this is a body').exists()).toBe(true); expect(wrapper.findByText('this is a header').exists()).toBe(true); + expect(wrapper.findByText('this is a header action').exists()).toBe(true); }); }); diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 93efce6b58b..1d5752ef284 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -471,7 +471,6 @@ RSpec.describe DiffHelper do describe '#conflicts' do let(:merge_request) { instance_double(MergeRequest, cannot_be_merged?: true) } - let(:merge_ref_head_diff) { true } let(:can_be_resolved_in_ui?) { true } let(:allow_tree_conflicts) { false } let(:files) { [instance_double(Gitlab::Conflict::File, path: 'a')] } @@ -479,7 +478,6 @@ RSpec.describe DiffHelper do before do allow(helper).to receive(:merge_request).and_return(merge_request) - allow(helper).to receive(:options).and_return(merge_ref_head_diff: merge_ref_head_diff) allow_next_instance_of(MergeRequests::Conflicts::ListService, merge_request, allow_tree_conflicts: allow_tree_conflicts) do |svc| allow(svc).to receive(:can_be_resolved_in_ui?).and_return(can_be_resolved_in_ui?) @@ -496,14 +494,6 @@ RSpec.describe DiffHelper do expect(helper.conflicts).to eq('a' => files.first) end - context 'when merge_ref_head_diff option is false' do - let(:merge_ref_head_diff) { false } - - it 'returns nil' do - expect(helper.conflicts).to be_nil - end - end - context 'when merge request can be merged' do let(:merge_request) { instance_double(MergeRequest, cannot_be_merged?: false) } diff --git a/spec/lib/gitlab/analytics/cycle_analytics/median_spec.rb b/spec/lib/gitlab/analytics/cycle_analytics/median_spec.rb index b4aa843bcd7..258f4a0d019 100644 --- a/spec/lib/gitlab/analytics/cycle_analytics/median_spec.rb +++ b/spec/lib/gitlab/analytics/cycle_analytics/median_spec.rb @@ -38,6 +38,6 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::Median do merge_request2.metrics.update!(merged_at: Time.zone.now) end - expect(subject).to be_within(0.5).of(7.5.minutes.seconds) + expect(subject).to be_within(5.seconds).of(7.5.minutes.seconds) end end diff --git a/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb index d88d9782021..16c5d7a4b6d 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do let(:default_branch) { 'master' } let(:pipeline_ref) { default_branch } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } - let(:pipeline) { service.execute!(:push).payload } + let(:pipeline) { service.execute(:push).payload } let(:build_names) { pipeline.builds.pluck(:name) } before do @@ -62,7 +62,8 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do context 'on master' do it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end @@ -70,7 +71,8 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do let(:pipeline_ref) { 'feature' } it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end @@ -78,7 +80,8 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do let(:pipeline_ref) { 'v1.0.0' } it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end end diff --git a/spec/lib/gitlab/ci/templates/Jobs/sast_iac_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/sast_iac_gitlab_ci_yaml_spec.rb index 85516d0bbb0..8a5aea7c0f0 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/sast_iac_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/sast_iac_gitlab_ci_yaml_spec.rb @@ -9,10 +9,10 @@ RSpec.describe 'Jobs/SAST-IaC.gitlab-ci.yml' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } - let(:default_branch) { 'main' } + let(:default_branch) { "master" } let(:pipeline_ref) { default_branch } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } - let(:pipeline) { service.execute!(:push).payload } + let(:pipeline) { service.execute(:push).payload } let(:build_names) { pipeline.builds.pluck(:name) } before do @@ -49,7 +49,8 @@ RSpec.describe 'Jobs/SAST-IaC.gitlab-ci.yml' do context 'on default branch' do it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end @@ -57,7 +58,8 @@ RSpec.describe 'Jobs/SAST-IaC.gitlab-ci.yml' do let(:pipeline_ref) { 'feature' } it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end end diff --git a/spec/lib/gitlab/ci/templates/Jobs/sast_iac_latest_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/sast_iac_latest_gitlab_ci_yaml_spec.rb index 5ff179b6fee..d540b035f81 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/sast_iac_latest_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/sast_iac_latest_gitlab_ci_yaml_spec.rb @@ -9,10 +9,10 @@ RSpec.describe 'Jobs/SAST-IaC.latest.gitlab-ci.yml' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } - let(:default_branch) { 'main' } + let(:default_branch) { "master" } let(:pipeline_ref) { default_branch } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } - let(:pipeline) { service.execute!(:push).payload } + let(:pipeline) { service.execute(:push).payload } let(:build_names) { pipeline.builds.pluck(:name) } before do @@ -50,7 +50,8 @@ RSpec.describe 'Jobs/SAST-IaC.latest.gitlab-ci.yml' do context 'on default branch' do it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end @@ -58,7 +59,8 @@ RSpec.describe 'Jobs/SAST-IaC.latest.gitlab-ci.yml' do let(:pipeline_ref) { 'feature' } it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end end diff --git a/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb index a92a8397e96..7cf0cf3ed33 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do let(:default_branch) { 'master' } let(:pipeline_ref) { default_branch } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } - let(:pipeline) { service.execute!(:push).payload } + let(:pipeline) { service.execute(:push).payload } let(:build_names) { pipeline.builds.pluck(:name) } before do @@ -62,7 +62,8 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do context 'on master' do it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end @@ -70,7 +71,8 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do let(:pipeline_ref) { 'feature' } it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end @@ -78,7 +80,8 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do let(:pipeline_ref) { 'v1.0.0' } it 'has no jobs' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end end diff --git a/spec/lib/gitlab/ci/templates/npm_spec.rb b/spec/lib/gitlab/ci/templates/npm_spec.rb index d86a3a67823..55fd4675f11 100644 --- a/spec/lib/gitlab/ci/templates/npm_spec.rb +++ b/spec/lib/gitlab/ci/templates/npm_spec.rb @@ -14,7 +14,7 @@ RSpec.describe 'npm.gitlab-ci.yml' do let(:pipeline_tag) { 'v1.2.1' } let(:pipeline_ref) { pipeline_branch } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref ) } - let(:pipeline) { service.execute!(:push).payload } + let(:pipeline) { service.execute(:push).payload } let(:build_names) { pipeline.builds.pluck(:name) } def create_branch(name:) @@ -42,7 +42,8 @@ RSpec.describe 'npm.gitlab-ci.yml' do shared_examples 'no pipeline created' do it 'does not create a pipeline because the only job (publish) is not created' do - expect { pipeline }.to raise_error(Ci::CreatePipelineService::CreateError, 'No stages / jobs for this pipeline.') + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end diff --git a/spec/lib/gitlab/ci/templates/themekit_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/themekit_gitlab_ci_yaml_spec.rb index 4708108f404..157fd39f1cc 100644 --- a/spec/lib/gitlab/ci/templates/themekit_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/themekit_gitlab_ci_yaml_spec.rb @@ -14,7 +14,7 @@ RSpec.describe 'ThemeKit.gitlab-ci.yml' do let(:project) { create(:project, :custom_repo, files: { 'README.md' => '' }) } let(:user) { project.first_owner } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } - let(:pipeline) { service.execute!(:push).payload } + let(:pipeline) { service.execute(:push).payload } let(:build_names) { pipeline.builds.pluck(:name) } before do @@ -51,9 +51,8 @@ RSpec.describe 'ThemeKit.gitlab-ci.yml' do end it 'has no jobs' do - expect { pipeline }.to raise_error( - Ci::CreatePipelineService::CreateError, 'No stages / jobs for this pipeline.' - ) + expect(build_names).to be_empty + expect(pipeline.errors.full_messages).to match_array(["No stages / jobs for this pipeline."]) end end end diff --git a/spec/lib/gitlab/sidekiq_migrate_jobs_spec.rb b/spec/lib/gitlab/sidekiq_migrate_jobs_spec.rb index db6ae1fc45a..c66e36c5621 100644 --- a/spec/lib/gitlab/sidekiq_migrate_jobs_spec.rb +++ b/spec/lib/gitlab/sidekiq_migrate_jobs_spec.rb @@ -382,5 +382,81 @@ RSpec.describe Gitlab::SidekiqMigrateJobs, :clean_gitlab_redis_queues do it_behaves_like 'migrating queues' end + + context 'when multiple workers are in the same queue' do + before do + ExportCsvWorker.sidekiq_options(queue: 'email_receiver') # follows EmailReceiverWorker's queue + ExportCsvWorker.perform_async('fizz') + end + + after do + ExportCsvWorker.set_queue + end + + context 'when the queue exists in mappings' do + let(:mappings) do + { 'EmailReceiverWorker' => 'email_receiver', 'AuthorizedProjectUpdate::ProjectRecalculateWorker' => 'default', + 'ExportCsvWorker' => 'default' } + end + + let(:queues_included_pre_migrate) do + ['email_receiver', + 'authorized_project_update:authorized_project_update_project_recalculate'] + end + + let(:queues_excluded_pre_migrate) { ['default'] } + let(:queues_excluded_post_migrate) do + ['authorized_project_update:authorized_project_update_project_recalculate'] + end + + let(:queues_included_post_migrate) { %w[default email_receiver] } + + it_behaves_like 'migrating queues' + def post_migrate_checks + # jobs from email_receiver are not migrated at all + jobs = list_jobs('email_receiver') + expect(jobs.length).to eq(3) + sorted = jobs.sort_by { |job| [job["class"], job["args"]] } + expect(sorted[0]).to include('class' => 'EmailReceiverWorker', 'args' => ['bar'], 'queue' => 'email_receiver') + expect(sorted[1]).to include('class' => 'EmailReceiverWorker', 'args' => ['foo'], 'queue' => 'email_receiver') + expect(sorted[2]).to include('class' => 'ExportCsvWorker', 'args' => ['fizz'], 'queue' => 'email_receiver') + end + end + + context 'when the queue doesnt exist in mappings' do + let(:mappings) do + { 'EmailReceiverWorker' => 'default', 'AuthorizedProjectUpdate::ProjectRecalculateWorker' => 'default', + 'ExportCsvWorker' => 'default' } + end + + let(:queues_included_pre_migrate) do + ['email_receiver', + 'authorized_project_update:authorized_project_update_project_recalculate'] + end + + let(:queues_excluded_pre_migrate) { ['default'] } + let(:queues_excluded_post_migrate) do + ['email_receiver', 'authorized_project_update:authorized_project_update_project_recalculate'] + end + + let(:queues_included_post_migrate) { ['default'] } + + it_behaves_like 'migrating queues' + def post_migrate_checks + # jobs from email_receiver are all migrated + jobs = list_jobs('email_receiver') + expect(jobs.length).to eq(0) + + jobs = list_jobs('default') + expect(jobs.length).to eq(4) + sorted = jobs.sort_by { |job| [job["class"], job["args"]] } + expect(sorted[0]).to include('class' => 'AuthorizedProjectUpdate::ProjectRecalculateWorker', + 'queue' => 'default') + expect(sorted[1]).to include('class' => 'EmailReceiverWorker', 'args' => ['bar'], 'queue' => 'default') + expect(sorted[2]).to include('class' => 'EmailReceiverWorker', 'args' => ['foo'], 'queue' => 'default') + expect(sorted[3]).to include('class' => 'ExportCsvWorker', 'args' => ['fizz'], 'queue' => 'default') + end + end + end end end diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index affec7864e9..a12bc7f4831 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -3,142 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Slack do - it_behaves_like Integrations::SlackMattermostNotifier, "Slack" - - describe '#execute' do - let_it_be(:project) { create(:project, :repository, :wiki_repo) } - let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all', project: project) } - - before do - stub_request(:post, slack_integration.webhook) - end - - it 'uses only known events', :aggregate_failures do - described_class::SUPPORTED_EVENTS_FOR_USAGE_LOG.each do |action| - expect(Gitlab::UsageDataCounters::HLLRedisCounter.known_event?("i_ecosystem_slack_service_#{action}_notification")).to be true - end - end - - context 'hook data includes a user object' do - let_it_be(:user) { create_default(:user) } - - shared_examples 'increases the usage data counter' do |event_name| - subject(:execute) { slack_integration.execute(data) } - - it 'increases the usage data counter' do - expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(event_name, values: user.id).and_call_original - - execute - end - - it_behaves_like 'Snowplow event tracking' do - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } - let(:category) { 'Integrations::Slack' } - let(:action) { 'perform_integrations_action' } - let(:namespace) { project.namespace } - let(:label) { 'redis_hll_counters.ecosystem.ecosystem_total_unique_counts_monthly' } - let(:property) { event_name } - end - end - - context 'event is not supported for usage log' do - let_it_be(:pipeline) { create(:ci_pipeline, project: project) } - - let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } - - it 'does not increase the usage data counter' do - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event).with('i_ecosystem_slack_service_pipeline_notification', values: user.id) - - slack_integration.execute(data) - end - end - - context 'issue notification' do - let_it_be(:issue) { create(:issue, project: project) } - - let(:data) { issue.to_hook_data(user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_issue_notification' - end - - context 'push notification' do - let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_push_notification' - end - - context 'deployment notification' do - let_it_be(:deployment) { create(:deployment, project: project, user: user) } - - let(:data) { Gitlab::DataBuilder::Deployment.build(deployment, deployment.status, Time.current) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_deployment_notification' - end - - context 'wiki_page notification' do - let(:wiki_page) { create(:wiki_page, wiki: project.wiki, project: project, message: 'user created page: Awesome wiki_page') } - - let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } - - before do - # Skip this method that is not relevant to this test to prevent having - # to update project which is frozen - allow(project.wiki).to receive(:after_wiki_activity) - end - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_wiki_page_notification' - end - - context 'merge_request notification' do - let_it_be(:merge_request) { create(:merge_request, source_project: project) } - - let(:data) { merge_request.to_hook_data(user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_merge_request_notification' - end - - context 'note notification' do - let_it_be(:issue_note) { create(:note_on_issue, project: project, note: 'issue note') } - - let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_note_notification' - end - - context 'tag_push notification' do - let(:oldrev) { Gitlab::Git::BLANK_SHA } - let(:newrev) { '8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b' } # gitlab-test: git rev-parse refs/tags/v1.1.0 - let(:ref) { 'refs/tags/v1.1.0' } - let(:data) { Git::TagHooksService.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }).send(:push_data) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_tag_push_notification' - end - - context 'confidential note notification' do - let_it_be(:confidential_issue_note) { create(:note_on_issue, project: project, note: 'issue note', confidential: true) } - - let(:data) { Gitlab::DataBuilder::Note.build(confidential_issue_note, user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_note_notification' - end - - context 'confidential issue notification' do - let_it_be(:issue) { create(:issue, project: project, confidential: true) } - - let(:data) { issue.to_hook_data(user) } - - it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_issue_notification' - end - end - - context 'hook data does not include a user' do - let(:data) { Gitlab::DataBuilder::Pipeline.build(create(:ci_pipeline, project: project)) } - - it 'does not increase the usage data counter' do - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) - - slack_integration.execute(data) - end - end - end + it_behaves_like Integrations::SlackMattermostNotifier, 'Slack' + it_behaves_like Integrations::BaseSlackNotification, factory: :integrations_slack end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 665e62b82b9..8fd223d241a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -5138,17 +5138,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end it 'returns false' do - expect(merge_request.diffable_merge_ref?).to eq(true) - end - - context 'display_merge_conflicts_in_diff is disabled' do - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it 'returns false' do - expect(merge_request.diffable_merge_ref?).to eq(false) - end + expect(merge_request.diffable_merge_ref?).to eq(false) end end end diff --git a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb index c859e91e21a..ec65e8cf11e 100644 --- a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb +++ b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb @@ -35,7 +35,7 @@ RSpec.describe 'Merge Requests Context Commit Diffs' do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) diff --git a/spec/requests/projects/merge_requests/diffs_spec.rb b/spec/requests/projects/merge_requests/diffs_spec.rb index 57548a3c83f..12990b54617 100644 --- a/spec/requests/projects/merge_requests/diffs_spec.rb +++ b/spec/requests/projects/merge_requests/diffs_spec.rb @@ -33,7 +33,7 @@ RSpec.describe 'Merge Requests Diffs' do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -128,7 +128,7 @@ RSpec.describe 'Merge Requests Diffs' do context 'with disabled display_merge_conflicts_in_diff feature' do let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(total_pages: 20).merge(allow_tree_conflicts: false) } + let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) } before do stub_feature_flags(display_merge_conflicts_in_diff: false) diff --git a/spec/serializers/diff_file_entity_spec.rb b/spec/serializers/diff_file_entity_spec.rb index 48099cb1fdf..fbb45162136 100644 --- a/spec/serializers/diff_file_entity_spec.rb +++ b/spec/serializers/diff_file_entity_spec.rb @@ -80,47 +80,13 @@ RSpec.describe DiffFileEntity do end end - describe '#is_fully_expanded' do - context 'file with a conflict' do - let(:options) { { conflicts: { diff_file.new_path => double(diff_lines_for_serializer: [], conflict_type: :both_modified) } } } - - it 'returns false' do - expect(diff_file).not_to receive(:fully_expanded?) - expect(subject[:is_fully_expanded]).to eq(false) - end - end - end - describe '#highlighted_diff_lines' do - context 'file without a conflict' do - let(:options) { { conflicts: {} } } + let(:options) { { conflicts: {} } } - it 'calls diff_lines_for_serializer on diff_file' do - # #diff_lines_for_serializer gets called in #fully_expanded? as well so we expect twice - expect(diff_file).to receive(:diff_lines_for_serializer).twice.and_return([]) - expect(subject[:highlighted_diff_lines]).to eq([]) - end - end - - context 'file with a conflict' do - let(:conflict_file) { instance_double(Gitlab::Conflict::File, conflict_type: :both_modified) } - let(:options) { { conflicts: { diff_file.new_path => conflict_file } } } - - it 'calls diff_lines_for_serializer on matching conflict file' do - expect(conflict_file).to receive(:diff_lines_for_serializer).and_return([]) - expect(subject[:highlighted_diff_lines]).to eq([]) - end - - context 'when Gitlab::Git::Conflict::Parser::UnmergeableFile gets raised' do - before do - allow(conflict_file).to receive(:diff_lines_for_serializer).and_raise(Gitlab::Git::Conflict::Parser::UnmergeableFile) - end - - it 'falls back to diff_file diff_lines_for_serializer' do - expect(diff_file).to receive(:diff_lines_for_serializer).and_return([]) - expect(subject[:highlighted_diff_lines]).to eq([]) - end - end + it 'calls diff_lines_for_serializer on diff_file' do + # #diff_lines_for_serializer gets called in #fully_expanded? as well so we expect twice + expect(diff_file).to receive(:diff_lines_for_serializer).twice.and_return([]) + expect(subject[:highlighted_diff_lines]).to eq([]) end end diff --git a/spec/serializers/diffs_entity_spec.rb b/spec/serializers/diffs_entity_spec.rb index 72777bde30c..ba40d538ccb 100644 --- a/spec/serializers/diffs_entity_spec.rb +++ b/spec/serializers/diffs_entity_spec.rb @@ -9,13 +9,13 @@ RSpec.describe DiffsEntity do let(:request) { EntityRequest.new(project: project, current_user: user) } let(:merge_request_diffs) { merge_request.merge_request_diffs } - let(:allow_tree_conflicts) { false } + let(:merge_conflicts_in_diff) { false } let(:options) do { request: request, merge_request: merge_request, merge_request_diffs: merge_request_diffs, - allow_tree_conflicts: allow_tree_conflicts + merge_conflicts_in_diff: merge_conflicts_in_diff } end @@ -87,60 +87,39 @@ RSpec.describe DiffsEntity do end end - context 'when there are conflicts' do + describe 'diff_files' do let(:diff_files) { merge_request_diffs.first.diffs.diff_files } - let(:diff_file_with_conflict) { diff_files.to_a.last } - let(:diff_file_without_conflict) { diff_files.to_a[-2] } - let(:resolvable_conflicts) { true } - let(:conflict_file) { double(path: diff_file_with_conflict.new_path, conflict_type: :both_modified) } - let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: resolvable_conflicts) } + it 'serializes diff files using DiffFileEntity' do + expect(DiffFileEntity) + .to receive(:represent) + .with( + diff_files, + hash_including(options.merge(conflicts: nil)) + ) - let(:merge_ref_head_diff) { true } - let(:options) { super().merge(merge_ref_head_diff: merge_ref_head_diff) } - - before do - allow(merge_request).to receive(:cannot_be_merged?).and_return(true) - allow(MergeRequests::Conflicts::ListService).to receive(:new).and_return(conflicts) + subject[:diff_files] end - it 'conflicts are highlighted' do - expect(conflict_file).to receive(:diff_lines_for_serializer) - expect(diff_file_with_conflict).not_to receive(:diff_lines_for_serializer) - expect(diff_file_without_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded + context 'when merge_conflicts_in_diff is true' do + let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) } + let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } + let(:merge_conflicts_in_diff) { true } - subject - end - - context 'merge ref head diff is not chosen to be displayed' do - let(:merge_ref_head_diff) { false } - - it 'conflicts are not calculated' do - expect(MergeRequests::Conflicts::ListService).not_to receive(:new) - end - end - - context 'when conflicts cannot be resolved' do - let(:resolvable_conflicts) { false } - - it 'conflicts are not highlighted' do - expect(conflict_file).not_to receive(:diff_lines_for_serializer) - expect(diff_file_with_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded - expect(diff_file_without_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded - - subject + before do + allow(merge_request).to receive(:cannot_be_merged?).and_return(true) + allow(MergeRequests::Conflicts::ListService).to receive(:new).and_return(conflicts) end - context 'when allow_tree_conflicts is set to true' do - let(:allow_tree_conflicts) { true } + it 'serializes diff files with conflicts' do + expect(DiffFileEntity) + .to receive(:represent) + .with( + diff_files, + hash_including(options.merge(conflicts: { conflict_file.path => conflict_file })) + ) - it 'conflicts are still highlighted' do - expect(conflict_file).to receive(:diff_lines_for_serializer) - expect(diff_file_with_conflict).not_to receive(:diff_lines_for_serializer) - expect(diff_file_without_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded - - subject - end + subject[:diff_files] end end end diff --git a/spec/serializers/diffs_metadata_entity_spec.rb b/spec/serializers/diffs_metadata_entity_spec.rb index 0e3d808aaac..04db576ffb5 100644 --- a/spec/serializers/diffs_metadata_entity_spec.rb +++ b/spec/serializers/diffs_metadata_entity_spec.rb @@ -9,6 +9,7 @@ RSpec.describe DiffsMetadataEntity do let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request_diffs) { merge_request.merge_request_diffs } let(:merge_request_diff) { merge_request_diffs.last } + let(:merge_conflicts_in_diff) { false } let(:options) { {} } let(:entity) do @@ -17,7 +18,8 @@ RSpec.describe DiffsMetadataEntity do options.merge( request: request, merge_request: merge_request, - merge_request_diffs: merge_request_diffs + merge_request_diffs: merge_request_diffs, + merge_conflicts_in_diff: merge_conflicts_in_diff ) ) end @@ -54,49 +56,36 @@ RSpec.describe DiffsMetadataEntity do end end - it 'returns diff files metadata' do - payload = DiffFileMetadataEntity.represent(raw_diff_files).as_json + it 'serializes diff files metadata using DiffFileMetadataEntity' do + expect(DiffFileMetadataEntity) + .to receive(:represent) + .with( + raw_diff_files, + hash_including(options.merge(conflicts: nil)) + ) - expect(subject[:diff_files]).to eq(payload) + subject[:diff_files] end - context 'when merge_ref_head_diff and allow_tree_conflicts options are set' do + context 'when merge_conflicts_in_diff is true' do let(:conflict_file) { double(path: raw_diff_files.first.new_path, conflict_type: :both_modified) } let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } + let(:merge_conflicts_in_diff) { true } before do allow(merge_request).to receive(:cannot_be_merged?).and_return(true) allow(MergeRequests::Conflicts::ListService).to receive(:new).and_return(conflicts) end - context 'when merge_ref_head_diff is true and allow_tree_conflicts is false' do - let(:options) { { merge_ref_head_diff: true, allow_tree_conflicts: false } } + it 'serializes diff files with conflicts' do + expect(DiffFileMetadataEntity) + .to receive(:represent) + .with( + raw_diff_files, + hash_including(options.merge(conflicts: { conflict_file.path => conflict_file })) + ) - it 'returns diff files metadata without conflicts' do - payload = DiffFileMetadataEntity.represent(raw_diff_files).as_json - - expect(subject[:diff_files]).to eq(payload) - end - end - - context 'when merge_ref_head_diff is false and allow_tree_conflicts is true' do - let(:options) { { merge_ref_head_diff: false, allow_tree_conflicts: true } } - - it 'returns diff files metadata without conflicts' do - payload = DiffFileMetadataEntity.represent(raw_diff_files).as_json - - expect(subject[:diff_files]).to eq(payload) - end - end - - context 'when merge_ref_head_diff and allow_tree_conflicts are true' do - let(:options) { { merge_ref_head_diff: true, allow_tree_conflicts: true } } - - it 'returns diff files metadata with conflicts' do - payload = DiffFileMetadataEntity.represent(raw_diff_files, conflicts: { conflict_file.path => conflict_file }).as_json - - expect(subject[:diff_files]).to eq(payload) - end + subject[:diff_files] end end end diff --git a/spec/serializers/paginated_diff_entity_spec.rb b/spec/serializers/paginated_diff_entity_spec.rb index 9d4456c11d6..3d77beb9abc 100644 --- a/spec/serializers/paginated_diff_entity_spec.rb +++ b/spec/serializers/paginated_diff_entity_spec.rb @@ -7,13 +7,13 @@ RSpec.describe PaginatedDiffEntity do let(:request) { double('request', current_user: user) } let(:merge_request) { create(:merge_request) } let(:diff_batch) { merge_request.merge_request_diff.diffs_in_batch(2, 3, diff_options: nil) } - let(:allow_tree_conflicts) { false } + let(:merge_conflicts_in_diff) { false } let(:options) do { request: request, merge_request: merge_request, pagination_data: diff_batch.pagination_data, - allow_tree_conflicts: allow_tree_conflicts + merge_conflicts_in_diff: merge_conflicts_in_diff } end @@ -29,61 +29,39 @@ RSpec.describe PaginatedDiffEntity do expect(subject[:pagination]).to eq(total_pages: 20) end - context 'when there are conflicts' do - let(:diff_batch) { merge_request.merge_request_diff.diffs_in_batch(7, 3, diff_options: nil) } - let(:diff_files) { diff_batch.diff_files.to_a } - let(:diff_file_with_conflict) { diff_files.last } - let(:diff_file_without_conflict) { diff_files.first } + describe 'diff_files' do + let(:diff_files) { diff_batch.diff_files(sorted: true) } - let(:resolvable_conflicts) { true } - let(:conflict_file) { double(path: diff_file_with_conflict.new_path, conflict_type: :both_modified) } - let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: resolvable_conflicts) } + it 'serializes diff files using DiffFileEntity' do + expect(DiffFileEntity) + .to receive(:represent) + .with( + diff_files, + hash_including(options.merge(conflicts: nil)) + ) - let(:merge_ref_head_diff) { true } - let(:options) { super().merge(merge_ref_head_diff: merge_ref_head_diff) } - - before do - allow(merge_request).to receive(:cannot_be_merged?).and_return(true) - allow(MergeRequests::Conflicts::ListService).to receive(:new).and_return(conflicts) + subject[:diff_files] end - it 'conflicts are highlighted' do - expect(conflict_file).to receive(:diff_lines_for_serializer) - expect(diff_file_with_conflict).not_to receive(:diff_lines_for_serializer) - expect(diff_file_without_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded + context 'when merge_conflicts_in_diff is true' do + let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) } + let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } + let(:merge_conflicts_in_diff) { true } - subject - end - - context 'merge ref head diff is not chosen to be displayed' do - let(:merge_ref_head_diff) { false } - - it 'conflicts are not calculated' do - expect(MergeRequests::Conflicts::ListService).not_to receive(:new) - end - end - - context 'when conflicts cannot be resolved' do - let(:resolvable_conflicts) { false } - - it 'conflicts are not highlighted' do - expect(conflict_file).not_to receive(:diff_lines_for_serializer) - expect(diff_file_with_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded - expect(diff_file_without_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded - - subject + before do + allow(merge_request).to receive(:cannot_be_merged?).and_return(true) + allow(MergeRequests::Conflicts::ListService).to receive(:new).and_return(conflicts) end - context 'when allow_tree_conflicts is set to true' do - let(:allow_tree_conflicts) { true } + it 'serializes diff files with conflicts' do + expect(DiffFileEntity) + .to receive(:represent) + .with( + diff_files, + hash_including(options.merge(conflicts: { conflict_file.path => conflict_file })) + ) - it 'conflicts are still highlighted' do - expect(conflict_file).to receive(:diff_lines_for_serializer) - expect(diff_file_with_conflict).not_to receive(:diff_lines_for_serializer) - expect(diff_file_without_conflict).to receive(:diff_lines_for_serializer).twice # for highlighted_diff_lines and is_fully_expanded - - subject - end + subject[:diff_files] end end end diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index c24b83e21a6..ee23238314e 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -190,14 +190,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar target_branch: 'conflict-start') end - it 'does not change the merge ref HEAD' do - expect(merge_request.merge_ref_head).to be_nil - - subject - - expect(merge_request.reload.merge_ref_head).not_to be_nil - end - it 'returns ServiceResponse.error and keeps merge status as cannot_be_merged' do result = subject @@ -351,27 +343,5 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar end end end - - context 'merge with conflicts' do - it 'calls MergeToRefService with true allow_conflicts param' do - expect(MergeRequests::MergeToRefService).to receive(:new) - .with(project: project, current_user: merge_request.author, params: { allow_conflicts: true }).and_call_original - - subject - end - - context 'when display_merge_conflicts_in_diff is disabled' do - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it 'calls MergeToRefService with false allow_conflicts param' do - expect(MergeRequests::MergeToRefService).to receive(:new) - .with(project: project, current_user: merge_request.author, params: { allow_conflicts: false }).and_call_original - - subject - end - end - end end end diff --git a/spec/support/shared_examples/models/concerns/integrations/base_slack_notification_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/base_slack_notification_shared_examples.rb new file mode 100644 index 00000000000..f0581333b28 --- /dev/null +++ b/spec/support/shared_examples/models/concerns/integrations/base_slack_notification_shared_examples.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +RSpec.shared_examples Integrations::BaseSlackNotification do |factory:| + describe '#execute' do + let_it_be(:project) { create(:project, :repository, :wiki_repo) } + let_it_be(:integration) { create(factory, branches_to_be_notified: 'all', project: project) } + + before do + stub_request(:post, integration.webhook) + end + + it 'uses only known events', :aggregate_failures do + described_class::SUPPORTED_EVENTS_FOR_USAGE_LOG.each do |action| + expect( + Gitlab::UsageDataCounters::HLLRedisCounter.known_event?("i_ecosystem_slack_service_#{action}_notification") + ).to be true + end + end + + context 'when hook data includes a user object' do + let_it_be(:user) { create_default(:user) } + + shared_examples 'increases the usage data counter' do |event_name| + subject(:execute) { integration.execute(data) } + + it 'increases the usage data counter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event).with(event_name, values: user.id).and_call_original + + execute + end + + it_behaves_like 'Snowplow event tracking' do + let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } + let(:category) { described_class.to_s } + let(:action) { 'perform_integrations_action' } + let(:namespace) { project.namespace } + let(:label) { 'redis_hll_counters.ecosystem.ecosystem_total_unique_counts_monthly' } + let(:property) { event_name } + end + end + + context 'when event is not supported for usage log' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + it 'does not increase the usage data counter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .not_to receive(:track_event).with('i_ecosystem_slack_service_pipeline_notification', values: user.id) + + integration.execute(data) + end + end + + context 'for issue notification' do + let_it_be(:issue) { create(:issue, project: project) } + + let(:data) { issue.to_hook_data(user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_issue_notification' + end + + context 'for push notification' do + let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_push_notification' + end + + context 'for deployment notification' do + let_it_be(:deployment) { create(:deployment, project: project, user: user) } + + let(:data) { Gitlab::DataBuilder::Deployment.build(deployment, deployment.status, Time.current) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_deployment_notification' + end + + context 'for wiki_page notification' do + let_it_be(:wiki_page) do + create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') + end + + let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } + + before do + # Skip this method that is not relevant to this test to prevent having + # to update project which is frozen + allow(project.wiki).to receive(:after_wiki_activity) + end + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_wiki_page_notification' + end + + context 'for merge_request notification' do + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + + let(:data) { merge_request.to_hook_data(user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_merge_request_notification' + end + + context 'for note notification' do + let_it_be(:issue_note) { create(:note_on_issue, project: project, note: 'issue note') } + + let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_note_notification' + end + + context 'for tag_push notification' do + let(:oldrev) { Gitlab::Git::BLANK_SHA } + let(:newrev) { '8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b' } # gitlab-test: git rev-parse refs/tags/v1.1.0 + let(:ref) { 'refs/tags/v1.1.0' } + let(:data) do + Git::TagHooksService.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }).send(:push_data) + end + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_tag_push_notification' + end + + context 'for confidential note notification' do + let_it_be(:confidential_issue_note) do + create(:note_on_issue, project: project, note: 'issue note', confidential: true) + end + + let(:data) { Gitlab::DataBuilder::Note.build(confidential_issue_note, user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_note_notification' + end + + context 'for confidential issue notification' do + let_it_be(:issue) { create(:issue, project: project, confidential: true) } + + let(:data) { issue.to_hook_data(user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_issue_notification' + end + end + + context 'when hook data does not include a user' do + let(:data) { Gitlab::DataBuilder::Pipeline.build(create(:ci_pipeline, project: project)) } + + it 'does not increase the usage data counter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + integration.execute(data) + end + end + end +end diff --git a/spec/workers/namespaces/root_statistics_worker_spec.rb b/spec/workers/namespaces/root_statistics_worker_spec.rb index f28a0815025..30854415405 100644 --- a/spec/workers/namespaces/root_statistics_worker_spec.rb +++ b/spec/workers/namespaces/root_statistics_worker_spec.rb @@ -90,6 +90,11 @@ RSpec.describe Namespaces::RootStatisticsWorker, '#perform' do end end + it_behaves_like 'worker with data consistency', + described_class, + feature_flag: :root_statistics_worker_read_replica, + data_consistency: :sticky + it 'has the `until_executed` deduplicate strategy' do expect(described_class.get_deduplicate_strategy).to eq(:until_executed) end