diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-25 21:09:23 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-25 21:09:23 +0000 |
| commit | 32fd4cd5e2134511936899d6bcc4aaf18b9be6fd (patch) | |
| tree | 10378ceffed52dd0e160a0d9bcf3c5ab72c18958 | |
| parent | 951616a26a61e880860ad862c1d45a8e3762b4bc (diff) | |
| download | gitlab-ce-32fd4cd5e2134511936899d6bcc4aaf18b9be6fd.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
67 files changed, 917 insertions, 523 deletions
@@ -237,7 +237,7 @@ gem 'atlassian-jwt', '~> 0.2.0' gem 'flowdock', '~> 0.7' # Slack integration -gem 'slack-notifier', '~> 1.5.1' +gem 'slack-messenger', '~> 2.3.3' # Hangouts Chat integration gem 'hangouts-chat', '~> 0.0.5' @@ -301,7 +301,7 @@ gem 'sentry-raven', '~> 2.9' gem 'premailer-rails', '~> 1.10.3' # LabKit: Tracing and Correlation -gem 'gitlab-labkit', '0.9.1' +gem 'gitlab-labkit', '0.10.0' # I18n gem 'ruby_parser', '~> 3.8', require: false diff --git a/Gemfile.lock b/Gemfile.lock index f90507ba23a..f4a0ae4ebef 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -380,7 +380,7 @@ GEM github-markup (1.7.0) gitlab-chronic (0.10.5) numerizer (~> 0.2) - gitlab-labkit (0.9.1) + gitlab-labkit (0.10.0) actionpack (>= 5.0.0, < 6.1.0) activesupport (>= 5.0.0, < 6.1.0) grpc (~> 1.19) @@ -1021,7 +1021,7 @@ GEM simplecov-html (~> 0.10.0) simplecov-html (0.10.2) sixarm_ruby_unaccent (1.2.0) - slack-notifier (1.5.1) + slack-messenger (2.3.3) snowplow-tracker (0.6.1) contracts (~> 0.7, <= 0.11) spring (2.0.2) @@ -1233,7 +1233,7 @@ DEPENDENCIES gitaly (~> 1.86.0) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) - gitlab-labkit (= 0.9.1) + gitlab-labkit (= 0.10.0) gitlab-license (~> 1.0) gitlab-markup (~> 1.7.0) gitlab-net-dns (~> 0.9.1) @@ -1376,7 +1376,7 @@ DEPENDENCIES sidekiq-cron (~> 1.0) simple_po_parser (~> 1.1.2) simplecov (~> 0.16.1) - slack-notifier (~> 1.5.1) + slack-messenger (~> 2.3.3) snowplow-tracker (~> 0.6.1) spring (~> 2.0.0) spring-commands-rspec (~> 1.0.4) diff --git a/app/assets/javascripts/notes/components/discussion_actions.vue b/app/assets/javascripts/notes/components/discussion_actions.vue index 8ab31ef3448..251199f1778 100644 --- a/app/assets/javascripts/notes/components/discussion_actions.vue +++ b/app/assets/javascripts/notes/components/discussion_actions.vue @@ -73,7 +73,7 @@ export default { v-if="discussion.resolvable && shouldShowJumpToNextDiscussion" class="btn-group discussion-actions ml-sm-2" > - <jump-to-next-discussion-button /> + <jump-to-next-discussion-button :from-discussion-id="discussion.id" /> </div> </div> </template> diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue index 70e22db364b..515d513e3ee 100644 --- a/app/assets/javascripts/notes/components/discussion_counter.vue +++ b/app/assets/javascripts/notes/components/discussion_counter.vue @@ -39,7 +39,11 @@ export default { </script> <template> - <div v-if="resolvableDiscussionsCount > 0" class="line-resolve-all-container full-width-mobile"> + <div + v-if="resolvableDiscussionsCount > 0" + ref="discussionCounter" + class="line-resolve-all-container full-width-mobile" + > <div class="full-width-mobile d-flex d-sm-block"> <div :class="{ 'has-next-btn': hasNextButton }" class="line-resolve-all"> <span diff --git a/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue b/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue index 630d4fd89b1..e66abcfddbb 100644 --- a/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue +++ b/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue @@ -12,6 +12,12 @@ export default { GlTooltip: GlTooltipDirective, }, mixins: [discussionNavigation], + props: { + fromDiscussionId: { + type: String, + required: true, + }, + }, }; </script> @@ -22,7 +28,7 @@ export default { v-gl-tooltip class="btn btn-default discussion-next-btn" :title="s__('MergeRequests|Jump to next unresolved thread')" - @click="jumpToNextDiscussion" + @click="jumpToNextRelativeDiscussion(fromDiscussionId)" > <icon name="comment-next" /> </button> diff --git a/app/assets/javascripts/notes/mixins/discussion_navigation.js b/app/assets/javascripts/notes/mixins/discussion_navigation.js index e5066695403..08c7efd69a6 100644 --- a/app/assets/javascripts/notes/mixins/discussion_navigation.js +++ b/app/assets/javascripts/notes/mixins/discussion_navigation.js @@ -2,6 +2,86 @@ import { mapGetters, mapActions, mapState } from 'vuex'; import { scrollToElement } from '~/lib/utils/common_utils'; import eventHub from '../../notes/event_hub'; +/** + * @param {string} selector + * @returns {boolean} + */ +function scrollTo(selector) { + const el = document.querySelector(selector); + + if (el) { + scrollToElement(el); + return true; + } + + return false; +} + +/** + * @param {object} self Component instance with mixin applied + * @param {string} id Discussion id we are jumping to + */ +function diffsJump({ expandDiscussion }, id) { + const selector = `ul.notes[data-discussion-id="${id}"]`; + eventHub.$once('scrollToDiscussion', () => scrollTo(selector)); + expandDiscussion({ discussionId: id }); +} + +/** + * @param {object} self Component instance with mixin applied + * @param {string} id Discussion id we are jumping to + * @returns {boolean} + */ +function discussionJump({ expandDiscussion }, id) { + const selector = `div.discussion[data-discussion-id="${id}"]`; + expandDiscussion({ discussionId: id }); + return scrollTo(selector); +} + +/** + * @param {object} self Component instance with mixin applied + * @param {string} id Discussion id we are jumping to + */ +function switchToDiscussionsTabAndJumpTo(self, id) { + window.mrTabs.eventHub.$once('MergeRequestTabChange', () => { + setTimeout(() => discussionJump(self, id), 0); + }); + + window.mrTabs.tabShown('show'); +} + +/** + * @param {object} self Component instance with mixin applied + * @param {object} discussion Discussion we are jumping to + */ +function jumpToDiscussion(self, discussion) { + const { id, diff_discussion: isDiffDiscussion } = discussion; + if (id) { + const activeTab = window.mrTabs.currentAction; + + if (activeTab === 'diffs' && isDiffDiscussion) { + diffsJump(self, id); + } else if (activeTab === 'show') { + discussionJump(self, id); + } else { + switchToDiscussionsTabAndJumpTo(self, id); + } + } +} + +/** + * @param {object} self Component instance with mixin applied + * @param {function} fn Which function used to get the target discussion's id + * @param {string} [discussionId=this.currentDiscussionId] Current discussion id, will be null if discussions have not been traversed yet + */ +function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId) { + const isDiffView = window.mrTabs.currentAction === 'diffs'; + const targetId = fn(discussionId, isDiffView); + const discussion = self.getDiscussion(targetId); + jumpToDiscussion(self, discussion); + self.setCurrentDiscussionId(targetId); +} + export default { computed: { ...mapGetters([ @@ -16,76 +96,20 @@ export default { methods: { ...mapActions(['expandDiscussion', 'setCurrentDiscussionId']), - diffsJump(id) { - const selector = `ul.notes[data-discussion-id="${id}"]`; - - eventHub.$once('scrollToDiscussion', () => { - const el = document.querySelector(selector); - - if (el) { - scrollToElement(el); - - return true; - } - - return false; - }); - - this.expandDiscussion({ discussionId: id }); - }, - discussionJump(id) { - const selector = `div.discussion[data-discussion-id="${id}"]`; - - const el = document.querySelector(selector); - - this.expandDiscussion({ discussionId: id }); - - if (el) { - scrollToElement(el); - - return true; - } - - return false; - }, - - switchToDiscussionsTabAndJumpTo(id) { - window.mrTabs.eventHub.$once('MergeRequestTabChange', () => { - setTimeout(() => this.discussionJump(id), 0); - }); - - window.mrTabs.tabShown('show'); - }, - - jumpToDiscussion(discussion) { - const { id, diff_discussion: isDiffDiscussion } = discussion; - if (id) { - const activeTab = window.mrTabs.currentAction; - - if (activeTab === 'diffs' && isDiffDiscussion) { - this.diffsJump(id); - } else if (activeTab === 'show') { - this.discussionJump(id); - } else { - this.switchToDiscussionsTabAndJumpTo(id); - } - } - }, - jumpToNextDiscussion() { - this.handleDiscussionJump(this.nextUnresolvedDiscussionId); + handleDiscussionJump(this, this.nextUnresolvedDiscussionId); }, jumpToPreviousDiscussion() { - this.handleDiscussionJump(this.previousUnresolvedDiscussionId); + handleDiscussionJump(this, this.previousUnresolvedDiscussionId); }, - handleDiscussionJump(fn) { - const isDiffView = window.mrTabs.currentAction === 'diffs'; - const targetId = fn(this.currentDiscussionId, isDiffView); - const discussion = this.getDiscussion(targetId); - this.jumpToDiscussion(discussion); - this.setCurrentDiscussionId(targetId); + /** + * Go to the next discussion from the given discussionId + * @param {String} discussionId The id we are jumping from + */ + jumpToNextRelativeDiscussion(discussionId) { + handleDiscussionJump(this, this.nextUnresolvedDiscussionId, discussionId); }, }, }; diff --git a/app/assets/javascripts/registry/explorer/pages/details.vue b/app/assets/javascripts/registry/explorer/pages/details.vue index bc613db8672..bfb9b0f4688 100644 --- a/app/assets/javascripts/registry/explorer/pages/details.vue +++ b/app/assets/javascripts/registry/explorer/pages/details.vue @@ -199,10 +199,7 @@ export default { </script> <template> - <div - v-gl-resize-observer="handleResize" - class="my-3 position-absolute w-100 slide-enter-to-element" - > + <div v-gl-resize-observer="handleResize" class="my-3 w-100 slide-enter-to-element"> <div class="d-flex my-3 align-items-center"> <h4> <gl-sprintf :message="s__('ContainerRegistry|%{imageName} tags')"> diff --git a/app/assets/javascripts/registry/explorer/pages/index.vue b/app/assets/javascripts/registry/explorer/pages/index.vue index deefbfc40e0..19ae3bee640 100644 --- a/app/assets/javascripts/registry/explorer/pages/index.vue +++ b/app/assets/javascripts/registry/explorer/pages/index.vue @@ -3,7 +3,7 @@ export default {}; </script> <template> - <div class="position-relative"> + <div> <transition name="slide"> <router-view /> </transition> diff --git a/app/assets/javascripts/registry/explorer/pages/list.vue b/app/assets/javascripts/registry/explorer/pages/list.vue index 1dbc7cc2242..5f8f4d8df1e 100644 --- a/app/assets/javascripts/registry/explorer/pages/list.vue +++ b/app/assets/javascripts/registry/explorer/pages/list.vue @@ -78,7 +78,7 @@ export default { </script> <template> - <div class="position-absolute w-100 slide-enter-from-element"> + <div class="w-100 slide-enter-from-element"> <gl-empty-state v-if="config.characterError" :title="s__('ContainerRegistry|Docker connection error')" diff --git a/app/assets/stylesheets/framework/vue_transitions.scss b/app/assets/stylesheets/framework/vue_transitions.scss index a082cd25abe..1a536b97142 100644 --- a/app/assets/stylesheets/framework/vue_transitions.scss +++ b/app/assets/stylesheets/framework/vue_transitions.scss @@ -15,6 +15,7 @@ .slide-enter-from-element { &.slide-enter, &.slide-leave-to { + position: absolute; transform: translateX(-150%); } } @@ -22,6 +23,7 @@ .slide-enter-to-element { &.slide-enter, &.slide-leave-to { + position: absolute; transform: translateX(150%); } } diff --git a/app/controllers/explore/snippets_controller.rb b/app/controllers/explore/snippets_controller.rb index 61068df77d1..3a56a48e578 100644 --- a/app/controllers/explore/snippets_controller.rb +++ b/app/controllers/explore/snippets_controller.rb @@ -1,17 +1,15 @@ # frozen_string_literal: true class Explore::SnippetsController < Explore::ApplicationController - include PaginatedCollection include Gitlab::NoteableMetadata def index @snippets = SnippetsFinder.new(current_user, explore: true) .execute .page(params[:page]) + .without_count .inc_author - return if redirect_out_of_range(@snippets) - @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet') end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d61f0bbfb10..f79a5682963 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -531,6 +531,7 @@ module Ci .concat(persisted_variables) .concat(scoped_variables) .concat(job_variables) + .concat(environment_changed_page_variables) .concat(persisted_environment_variables) .to_runner_variables end @@ -569,6 +570,15 @@ module Ci end end + def environment_changed_page_variables + Gitlab::Ci::Variables::Collection.new.tap do |variables| + break variables unless environment_status + + variables.append(key: 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS', value: environment_status.changed_paths.join(',')) + variables.append(key: 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS', value: environment_status.changed_urls.join(',')) + end + end + def deploy_token_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| break variables unless gitlab_deploy_token @@ -970,6 +980,14 @@ module Ci options&.dig(:environment, :url) || persisted_environment&.external_url end + def environment_status + strong_memoize(:environment_status) do + if has_environment? && merge_request + EnvironmentStatus.new(project, persisted_environment, merge_request, pipeline.sha) + end + end + end + # The format of the retry option changed in GitLab 11.5: Before it was # integer only, after it is a hash. New builds are created with the new # format, but builds created before GitLab 11.5 and saved in database still diff --git a/app/models/environment_status.rb b/app/models/environment_status.rb index 5fdb5af2d9b..46e41c22139 100644 --- a/app/models/environment_status.rb +++ b/app/models/environment_status.rb @@ -62,9 +62,9 @@ class EnvironmentStatus end def changes - return [] unless has_route_map? - - changed_files.map { |file| build_change(file) }.compact + strong_memoize(:changes) do + has_route_map? ? changed_files.map { |file| build_change(file) }.compact : [] + end end def changed_files @@ -72,6 +72,14 @@ class EnvironmentStatus .merge_request_diff_files.where(deleted_file: false) end + def changed_paths + changes.map { |change| change[:path] } + end + + def changed_urls + changes.map { |change| change[:external_url] } + end + def has_route_map? project.route_map_for(sha).present? end diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 814c970f745..37d45c5934d 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -68,6 +68,10 @@ class PagesDomain < ApplicationRecord scope :instance_serverless, -> { where(wildcard: true, scope: :instance, usage: :serverless) } + def self.find_by_domain_case_insensitive(domain) + find_by("LOWER(domain) = LOWER(?)", domain) + end + def verified? !!verified_at end diff --git a/app/models/project_services/chat_message/base_message.rb b/app/models/project_services/chat_message/base_message.rb index 49151ad24cb..bdd77a919e3 100644 --- a/app/models/project_services/chat_message/base_message.rb +++ b/app/models/project_services/chat_message/base_message.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'slack-notifier' - module ChatMessage class BaseMessage RELATIVE_LINK_REGEX = /!\[[^\]]*\]\((\/uploads\/[^\)]*)\)/.freeze @@ -59,7 +57,7 @@ module ChatMessage end def format(string) - Slack::Notifier::LinkFormatter.format(format_relative_links(string)) + Slack::Messenger::Util::LinkFormatter.format(format_relative_links(string)) end def format_relative_links(string) diff --git a/app/models/project_services/chat_message/pipeline_message.rb b/app/models/project_services/chat_message/pipeline_message.rb index 46fe894cfc3..52a26f6211a 100644 --- a/app/models/project_services/chat_message/pipeline_message.rb +++ b/app/models/project_services/chat_message/pipeline_message.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -require 'slack-notifier' module ChatMessage class PipelineMessage < BaseMessage @@ -98,7 +97,7 @@ module ChatMessage def failed_stages_field { title: s_("ChatMessage|Failed stage").pluralize(failed_stages.length), - value: Slack::Notifier::LinkFormatter.format(failed_stages_links), + value: Slack::Messenger::Util::LinkFormatter.format(failed_stages_links), short: true } end @@ -106,7 +105,7 @@ module ChatMessage def failed_jobs_field { title: s_("ChatMessage|Failed job").pluralize(failed_jobs.length), - value: Slack::Notifier::LinkFormatter.format(failed_jobs_links), + value: Slack::Messenger::Util::LinkFormatter.format(failed_jobs_links), short: true } end @@ -123,12 +122,12 @@ module ChatMessage fields = [ { title: ref_type == "tag" ? s_("ChatMessage|Tag") : s_("ChatMessage|Branch"), - value: Slack::Notifier::LinkFormatter.format(ref_link), + value: Slack::Messenger::Util::LinkFormatter.format(ref_link), short: true }, { title: s_("ChatMessage|Commit"), - value: Slack::Notifier::LinkFormatter.format(commit_link), + value: Slack::Messenger::Util::LinkFormatter.format(commit_link), short: true } ] diff --git a/app/models/project_services/chat_message/push_message.rb b/app/models/project_services/chat_message/push_message.rb index 41b0cbb2c4d..c8e70a69c88 100644 --- a/app/models/project_services/chat_message/push_message.rb +++ b/app/models/project_services/chat_message/push_message.rb @@ -48,7 +48,7 @@ module ChatMessage end def format(string) - Slack::Notifier::LinkFormatter.format(string) + Slack::Messenger::Util::LinkFormatter.format(string) end def commit_messages diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index 46c8260ab48..7bd011101dd 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -84,10 +84,10 @@ class ChatNotificationService < Service event_type = data[:event_type] || object_kind - channel_name = get_channel_field(event_type).presence || channel + channel_names = get_channel_field(event_type).presence || channel opts = {} - opts[:channel] = channel_name if channel_name + opts[:channel] = channel_names.split(',').map(&:strip) if channel_names opts[:username] = username if username return false unless notify(message, opts) diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 7290964f442..6d567bb1383 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -13,18 +13,8 @@ class SlackService < ChatNotificationService 'slack' end - def help - 'This service sends notifications about projects events to Slack channels.<br /> - To set up this service: - <ol> - <li><a href="https://slack.com/apps/A0F7XDUAZ-incoming-webhooks">Add an incoming webhook</a> in your Slack team. The default channel can be overridden for each event.</li> - <li>Paste the <strong>Webhook URL</strong> into the field below.</li> - <li>Select events below to enable notifications. The <strong>Channel name</strong> and <strong>Username</strong> fields are optional.</li> - </ol>' - end - def default_channel_placeholder - "Channel name (e.g. general)" + _('Slack channels (e.g. general, development)') end def webhook_placeholder @@ -35,8 +25,8 @@ class SlackService < ChatNotificationService private def notify(message, opts) - # See https://github.com/stevenosloan/slack-notifier#custom-http-client - notifier = Slack::Notifier.new(webhook, opts.merge(http_client: HTTPClient)) + # See https://gitlab.com/gitlab-org/slack-notifier/#custom-http-client + notifier = Slack::Messenger.new(webhook, opts.merge(http_client: HTTPClient)) notifier.ping( message.pretext, diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb index 6a454070fe2..01ded0495a7 100644 --- a/app/models/project_services/slack_slash_commands_service.rb +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -29,6 +29,6 @@ class SlackSlashCommandsService < SlashCommandsService private def format(text) - Slack::Notifier::LinkFormatter.format(text) if text + Slack::Messenger::Util::LinkFormatter.format(text) if text end end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 745700b1c65..8f162f17ac5 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -17,7 +17,7 @@ class Snippet < ApplicationRecord include HasRepository extend ::Gitlab::Utils::Override - ignore_column :repository_storage, remove_with: '12.10', remove_after: '2020-04-22' + ignore_column :repository_storage, remove_with: '12.10', remove_after: '2020-03-22' cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description diff --git a/app/views/projects/services/slack/_help.haml b/app/views/projects/services/slack/_help.haml new file mode 100644 index 00000000000..d7ea1b270f5 --- /dev/null +++ b/app/views/projects/services/slack/_help.haml @@ -0,0 +1,16 @@ +- webhooks_link_url = 'https://slack.com/apps/A0F7XDUAZ-incoming-webhooks' +- webhooks_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: webhooks_link_url } + +.info-well + .well-segment + %p= s_('SlackIntegration|This service send notifications about projects\' events to Slack channels. To set up this service:') + %ol + %li + = s_('SlackIntegration|%{webhooks_link_start}Add an incoming webhook%{webhooks_link_end} in your Slack team. The default channel can be overridden for each event.').html_safe % { webhooks_link_start: webhooks_link_start, webhooks_link_end: '</a>'.html_safe } + %li + = s_('SlackIntegration|Paste the <strong>Webhook URL</strong> into the field below.').html_safe + %li + = s_('SlackIntegration|Select events below to enable notifications. The <strong>Slack channel names</strong> and <strong>Slack username</strong> fields are optional.').html_safe + %p.mt-3.mb-0 + = s_('SlackIntegration|<strong>Note:</strong> Usernames and private channels are not supported.').html_safe + = link_to _('Learn more'), help_page_path('user/project/integrations/slack') diff --git a/app/views/shared/snippets/_list.html.haml b/app/views/shared/snippets/_list.html.haml index 766f48fff3d..ca3a291ae27 100644 --- a/app/views/shared/snippets/_list.html.haml +++ b/app/views/shared/snippets/_list.html.haml @@ -8,4 +8,4 @@ %ul.content-list = render partial: 'shared/snippets/snippet', collection: @snippets, locals: { link_project: link_project } - = paginate @snippets, theme: 'gitlab', remote: remote + = paginate_collection @snippets, remote: remote diff --git a/app/workers/concerns/waitable_worker.rb b/app/workers/concerns/waitable_worker.rb index 17946bbc5ca..f995aced542 100644 --- a/app/workers/concerns/waitable_worker.rb +++ b/app/workers/concerns/waitable_worker.rb @@ -9,7 +9,7 @@ module WaitableWorker # Short-circuit: it's more efficient to do small numbers of jobs inline return bulk_perform_inline(args_list) if args_list.size <= 3 - waiter = Gitlab::JobWaiter.new(args_list.size) + waiter = Gitlab::JobWaiter.new(args_list.size, worker_label: self.to_s) # Point all the bulk jobs at the same JobWaiter. Converts, [[1], [2], [3]] # into [[1, "key"], [2, "key"], [3, "key"]] diff --git a/changelogs/unreleased/14080-slack-multiple-channels.yml b/changelogs/unreleased/14080-slack-multiple-channels.yml new file mode 100644 index 00000000000..efc7299783b --- /dev/null +++ b/changelogs/unreleased/14080-slack-multiple-channels.yml @@ -0,0 +1,5 @@ +--- +title: Allow multiple Slack channels for notifications +merge_request: 24132 +author: +type: added diff --git a/changelogs/unreleased/207390-pages-api-case-insensitive-domain-lookup.yml b/changelogs/unreleased/207390-pages-api-case-insensitive-domain-lookup.yml new file mode 100644 index 00000000000..e2dd0f8b082 --- /dev/null +++ b/changelogs/unreleased/207390-pages-api-case-insensitive-domain-lookup.yml @@ -0,0 +1,5 @@ +--- +title: 'Add index on LOWER(domain) for pages_domains' +merge_request: 25664 +author: +type: other diff --git a/changelogs/unreleased/34420-optimize-pagination-on-explore-snippets.yml b/changelogs/unreleased/34420-optimize-pagination-on-explore-snippets.yml new file mode 100644 index 00000000000..48748c57a0c --- /dev/null +++ b/changelogs/unreleased/34420-optimize-pagination-on-explore-snippets.yml @@ -0,0 +1,5 @@ +--- +title: Showing only "Next" button for snippet explore page. +merge_request: 25404 +author: +type: changed diff --git a/changelogs/unreleased/changed-pages-ci-var.yml b/changelogs/unreleased/changed-pages-ci-var.yml new file mode 100644 index 00000000000..62da65a2ddd --- /dev/null +++ b/changelogs/unreleased/changed-pages-ci-var.yml @@ -0,0 +1,5 @@ +--- +title: Added CI_MERGE_REQUEST_CHANGED_PAGE_* to Predefined Variables reference +merge_request: 25256 +author: +type: added diff --git a/changelogs/unreleased/fj-remove-repository-storage-column-from-snippets.yml b/changelogs/unreleased/fj-remove-repository-storage-column-from-snippets.yml new file mode 100644 index 00000000000..b3cc3143fc4 --- /dev/null +++ b/changelogs/unreleased/fj-remove-repository-storage-column-from-snippets.yml @@ -0,0 +1,5 @@ +--- +title: Remove repository_storage column from snippets +merge_request: 25699 +author: +type: other diff --git a/changelogs/unreleased/jdb-fix-jump-to-next-unresolved-thread.yml b/changelogs/unreleased/jdb-fix-jump-to-next-unresolved-thread.yml new file mode 100644 index 00000000000..72d37699fee --- /dev/null +++ b/changelogs/unreleased/jdb-fix-jump-to-next-unresolved-thread.yml @@ -0,0 +1,5 @@ +--- +title: Fix Jump to next unresolved thread +merge_request: 24728 +author: +type: fixed diff --git a/config/initializers/tracing.rb b/config/initializers/tracing.rb index aaf74eb4cd3..f26fb18f3ea 100644 --- a/config/initializers/tracing.rb +++ b/config/initializers/tracing.rb @@ -5,22 +5,6 @@ if Labkit::Tracing.enabled? config.middleware.insert_after Labkit::Middleware::Rack, ::Labkit::Tracing::RackMiddleware end - # Instrument the Sidekiq client - Sidekiq.configure_client do |config| - config.client_middleware do |chain| - chain.add Labkit::Tracing::Sidekiq::ClientMiddleware - end - end - - # Instrument Sidekiq server calls when running Sidekiq server - if Gitlab::Runtime.sidekiq? - Sidekiq.configure_server do |config| - config.server_middleware do |chain| - chain.add Labkit::Tracing::Sidekiq::ServerMiddleware - end - end - end - # Instrument Redis Labkit::Tracing::Redis.instrument diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 1cb19d18a0d..d4bf346ede7 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -28,6 +28,8 @@ - 1 - - admin_emails - 1 +- - analytics_code_review_metrics + - 1 - - authorized_projects - 2 - - auto_devops diff --git a/db/migrate/20200221023320_add_index_on_pages_domain_on_domain_lowercase.rb b/db/migrate/20200221023320_add_index_on_pages_domain_on_domain_lowercase.rb new file mode 100644 index 00000000000..53f8f5e7f81 --- /dev/null +++ b/db/migrate/20200221023320_add_index_on_pages_domain_on_domain_lowercase.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddIndexOnPagesDomainOnDomainLowercase < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + INDEX_NAME = 'index_pages_domains_on_domain_lowercase' + + disable_ddl_transaction! + + def up + add_concurrent_index :pages_domains, 'LOWER(domain)', name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :pages_domains, INDEX_NAME + end +end diff --git a/db/migrate/20200221074028_add_mr_metrics_first_approved_at.rb b/db/migrate/20200221074028_add_mr_metrics_first_approved_at.rb new file mode 100644 index 00000000000..993905f66ce --- /dev/null +++ b/db/migrate/20200221074028_add_mr_metrics_first_approved_at.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddMrMetricsFirstApprovedAt < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + add_column :merge_request_metrics, :first_approved_at, :datetime_with_timezone + end + + def down + remove_column :merge_request_metrics, :first_approved_at + end +end diff --git a/db/post_migrate/20200221142216_remove_repository_storage_from_snippets.rb b/db/post_migrate/20200221142216_remove_repository_storage_from_snippets.rb new file mode 100644 index 00000000000..f9ef985218b --- /dev/null +++ b/db/post_migrate/20200221142216_remove_repository_storage_from_snippets.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class RemoveRepositoryStorageFromSnippets < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + return unless column_exists?(:snippets, :repository_storage) + + remove_column :snippets, :repository_storage + end + + def down + return if column_exists?(:snippets, :repository_storage) + + add_column_with_default( # rubocop:disable Migration/AddColumnWithDefault + :snippets, + :repository_storage, + :string, + default: 'default', + limit: 255, + allow_null: false + ) + end +end diff --git a/db/schema.rb b/db/schema.rb index 8052a93e841..36bf8ad5e01 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2537,6 +2537,7 @@ ActiveRecord::Schema.define(version: 2020_02_24_163804) do t.integer "diff_size" t.integer "modified_paths_size" t.integer "commits_count" + t.datetime_with_timezone "first_approved_at" t.index ["first_deployed_to_production_at"], name: "index_merge_request_metrics_on_first_deployed_to_production_at" t.index ["latest_closed_at"], name: "index_merge_request_metrics_on_latest_closed_at", where: "(latest_closed_at IS NOT NULL)" t.index ["latest_closed_by_id"], name: "index_merge_request_metrics_on_latest_closed_by_id" @@ -3062,6 +3063,7 @@ ActiveRecord::Schema.define(version: 2020_02_24_163804) do t.boolean "wildcard", default: false, null: false t.integer "usage", limit: 2, default: 0, null: false t.integer "scope", limit: 2, default: 2, null: false + t.index "lower((domain)::text)", name: "index_pages_domains_on_domain_lowercase" t.index ["certificate_source", "certificate_valid_not_after"], name: "index_pages_domains_need_auto_ssl_renewal", where: "(auto_ssl_enabled = true)" t.index ["domain", "wildcard"], name: "index_pages_domains_on_domain_and_wildcard", unique: true t.index ["project_id", "enabled_until"], name: "index_pages_domains_on_project_id_and_enabled_until" @@ -3938,7 +3940,6 @@ ActiveRecord::Schema.define(version: 2020_02_24_163804) do t.string "encrypted_secret_token", limit: 255 t.string "encrypted_secret_token_iv", limit: 255 t.boolean "secret", default: false, null: false - t.string "repository_storage", limit: 255, default: "default", null: false t.index ["author_id"], name: "index_snippets_on_author_id" t.index ["content"], name: "index_snippets_on_content_trigram", opclass: :gin_trgm_ops, using: :gin t.index ["created_at"], name: "index_snippets_on_created_at" diff --git a/doc/ci/variables/predefined_variables.md b/doc/ci/variables/predefined_variables.md index a9f57d97316..2bc9132aa93 100644 --- a/doc/ci/variables/predefined_variables.md +++ b/doc/ci/variables/predefined_variables.md @@ -64,6 +64,8 @@ future GitLab releases.** | `CI_JOB_TOKEN` | 9.0 | 1.2 | Token used for authenticating with the [GitLab Container Registry][registry] and downloading [dependent repositories][dependent-repositories] | | `CI_JOB_URL` | 11.1 | 0.5 | Job details URL | | `CI_MERGE_REQUEST_ASSIGNEES` | 11.9 | all | Comma-separated list of username(s) of assignee(s) for the merge request if [the pipelines are for merge requests](../merge_request_pipelines/index.md). Available only if `only: [merge_requests]` is used and the merge request is created. | +| `CI_MERGE_REQUEST_CHANGED_PAGE_PATHS` | 12.9 | all | Comma-separated list of paths of changed pages in a deployed [Review App](../review_apps/index.md) for a [Merge Request](../merge_request_pipelines/index.md). A [Route Map](../review_apps/index.md#route-maps) must be configured. | +| `CI_MERGE_REQUEST_CHANGED_PAGE_URLS` | 12.9 | all | Comma-separated list of URLs of changed pages in a deployed [Review App](../review_apps/index.md) for a [Merge Request](../merge_request_pipelines/index.md). A [Route Map](../review_apps/index.md#route-maps) must be configured. | | `CI_MERGE_REQUEST_ID` | 11.6 | all | The ID of the merge request if [the pipelines are for merge requests](../merge_request_pipelines/index.md). Available only if `only: [merge_requests]` is used and the merge request is created. | | `CI_MERGE_REQUEST_IID` | 11.6 | all | The IID of the merge request if [the pipelines are for merge requests](../merge_request_pipelines/index.md). Available only if `only: [merge_requests]` is used and the merge request is created. | | `CI_MERGE_REQUEST_LABELS` | 11.9 | all | Comma-separated label names of the merge request if [the pipelines are for merge requests](../merge_request_pipelines/index.md). Available only if `only: [merge_requests]` is used and the merge request is created. | diff --git a/doc/user/project/integrations/slack.md b/doc/user/project/integrations/slack.md index 1dda3a60430..9414dc8534c 100644 --- a/doc/user/project/integrations/slack.md +++ b/doc/user/project/integrations/slack.md @@ -16,7 +16,7 @@ The Slack Notifications Service allows your GitLab project to send events (e.g. 1. Select the **Slack notifications** project service to configure it. 1. Check the **Active** checkbox to turn on the service. 1. Check the checkboxes corresponding to the GitLab events you want to send to Slack as a notification. -1. For each event, optionally enter the Slack channel where you want to send the event. (Do _not_ include the `#` symbol.) If left empty, the event will be sent to the default channel that you configured in the Slack Configuration step. +1. For each event, optionally enter the Slack channel names where you want to send the event, separated by a comma. If left empty, the event will be sent to the default channel that you configured in the Slack Configuration step. **Note:** Usernames and private channels are not supported. To send direct messages, use the Member ID found under user's Slack profile. 1. Paste the **Webhook URL** that you copied from the Slack Configuration step. 1. Optionally customize the Slack bot username that will be sending the notifications. 1. Configure the remaining options and click `Save changes`. diff --git a/lib/api/internal/pages.rb b/lib/api/internal/pages.rb index e2e1351c939..4339d2ef490 100644 --- a/lib/api/internal/pages.rb +++ b/lib/api/internal/pages.rb @@ -36,7 +36,7 @@ module API present virtual_domain, with: Entities::Internal::Serverless::VirtualDomain else # Handle Pages domains - host = Namespace.find_by_pages_host(params[:host]) || PagesDomain.find_by_domain(params[:host]) + host = Namespace.find_by_pages_host(params[:host]) || PagesDomain.find_by_domain_case_insensitive(params[:host]) no_content! unless host virtual_domain = host.pages_virtual_domain diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index aa275479e63..4fa909ac94b 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -173,7 +173,6 @@ excluded_attributes: - :secret - :encrypted_secret_token - :encrypted_secret_token_iv - - :repository_storage merge_request_diff: - :external_diff - :stored_externally diff --git a/lib/gitlab/job_waiter.rb b/lib/gitlab/job_waiter.rb index 90dbe4d005d..e7a8cc6305a 100644 --- a/lib/gitlab/job_waiter.rb +++ b/lib/gitlab/job_waiter.rb @@ -19,6 +19,9 @@ module Gitlab class JobWaiter KEY_PREFIX = "gitlab:job_waiter" + STARTED_METRIC = :gitlab_job_waiter_started_total + TIMEOUTS_METRIC = :gitlab_job_waiter_timeouts_total + def self.notify(key, jid) Gitlab::Redis::SharedState.with { |redis| redis.lpush(key, jid) } end @@ -27,15 +30,16 @@ module Gitlab key.is_a?(String) && key =~ /\A#{KEY_PREFIX}:\h{8}-\h{4}-\h{4}-\h{4}-\h{12}\z/ end - attr_reader :key, :finished + attr_reader :key, :finished, :worker_label attr_accessor :jobs_remaining # jobs_remaining - the number of jobs left to wait for # key - The key of this waiter. - def initialize(jobs_remaining = 0, key = "#{KEY_PREFIX}:#{SecureRandom.uuid}") + def initialize(jobs_remaining = 0, key = "#{KEY_PREFIX}:#{SecureRandom.uuid}", worker_label: nil) @key = key @jobs_remaining = jobs_remaining @finished = [] + @worker_label = worker_label end # Waits for all the jobs to be completed. @@ -45,6 +49,7 @@ module Gitlab # long to process, or is never processed. def wait(timeout = 10) deadline = Time.now.utc + timeout + increment_counter(STARTED_METRIC) Gitlab::Redis::SharedState.with do |redis| # Fallback key expiry: allow a long grace period to reduce the chance of @@ -60,7 +65,12 @@ module Gitlab break if seconds_left <= 0 list, jid = redis.blpop(key, timeout: seconds_left) - break unless list && jid # timed out + + # timed out + unless list && jid + increment_counter(TIMEOUTS_METRIC) + break + end @finished << jid @jobs_remaining -= 1 @@ -72,5 +82,20 @@ module Gitlab finished end + + private + + def increment_counter(metric) + return unless worker_label + + metrics[metric].increment(worker: worker_label) + end + + def metrics + @metrics ||= { + STARTED_METRIC => Gitlab::Metrics.counter(STARTED_METRIC, 'JobWaiter attempts started'), + TIMEOUTS_METRIC => Gitlab::Metrics.counter(TIMEOUTS_METRIC, 'JobWaiter attempts timed out') + } + end end end diff --git a/lib/gitlab/slash_commands/presenters/base.rb b/lib/gitlab/slash_commands/presenters/base.rb index 54d74ed3998..08de9df14f8 100644 --- a/lib/gitlab/slash_commands/presenters/base.rb +++ b/lib/gitlab/slash_commands/presenters/base.rb @@ -63,7 +63,7 @@ module Gitlab # Convert Markdown to slacks format def format(string) - Slack::Notifier::LinkFormatter.format(string) + Slack::Messenger::Util::LinkFormatter.format(string) end def resource_url diff --git a/lib/gitlab/tracing.rb b/lib/gitlab/tracing.rb deleted file mode 100644 index 7732d7c9d9c..00000000000 --- a/lib/gitlab/tracing.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Tracing - # Only enable tracing when the `GITLAB_TRACING` env var is configured. Note that we avoid using ApplicationSettings since - # the same environment variable needs to be configured for Workhorse, Gitaly and any other components which - # emit tracing. Since other components may start before Rails, and may not have access to ApplicationSettings, - # an env var makes more sense. - def self.enabled? - connection_string.present? - end - - def self.connection_string - ENV['GITLAB_TRACING'] - end - - def self.tracing_url_template - ENV['GITLAB_TRACING_URL'] - end - - def self.tracing_url_enabled? - enabled? && tracing_url_template.present? - end - - # This will provide a link into the distributed tracing for the current trace, - # if it has been captured. - def self.tracing_url - return unless tracing_url_enabled? - - # Avoid using `format` since it can throw TypeErrors - # which we want to avoid on unsanitised env var input - tracing_url_template.to_s - .gsub(/\{\{\s*correlation_id\s*\}\}/, Labkit::Correlation::CorrelationId.current_id.to_s) - .gsub(/\{\{\s*service\s*\}\}/, Gitlab.process_name) - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ef30d868a01..1f6e1c9f441 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17847,9 +17847,27 @@ msgstr "" msgid "Slack application" msgstr "" +msgid "Slack channels (e.g. general, development)" +msgstr "" + msgid "Slack integration allows you to interact with GitLab via slash commands in a chat window." msgstr "" +msgid "SlackIntegration|%{webhooks_link_start}Add an incoming webhook%{webhooks_link_end} in your Slack team. The default channel can be overridden for each event." +msgstr "" + +msgid "SlackIntegration|<strong>Note:</strong> Usernames and private channels are not supported." +msgstr "" + +msgid "SlackIntegration|Paste the <strong>Webhook URL</strong> into the field below." +msgstr "" + +msgid "SlackIntegration|Select events below to enable notifications. The <strong>Slack channel names</strong> and <strong>Slack username</strong> fields are optional." +msgstr "" + +msgid "SlackIntegration|This service send notifications about projects' events to Slack channels. To set up this service:" +msgstr "" + msgid "SlackService|2. Paste the <strong>Token</strong> into the field below" msgstr "" diff --git a/spec/controllers/explore/snippets_controller_spec.rb b/spec/controllers/explore/snippets_controller_spec.rb index fa659c6df7f..ab91faa6cef 100644 --- a/spec/controllers/explore/snippets_controller_spec.rb +++ b/spec/controllers/explore/snippets_controller_spec.rb @@ -4,12 +4,33 @@ require 'spec_helper' describe Explore::SnippetsController do describe 'GET #index' do - it_behaves_like 'paginated collection' do - let(:collection) { Snippet.all } + let!(:project_snippet) { create_list(:project_snippet, 3, :public) } + let!(:personal_snippet) { create_list(:personal_snippet, 3, :public) } - before do - create(:personal_snippet, :public) - end + before do + allow(Kaminari.config).to receive(:default_per_page).and_return(2) + end + + it 'renders' do + get :index + + snippets = assigns(:snippets) + + expect(snippets).to be_a(::Kaminari::PaginatableWithoutCount) + expect(snippets.size).to eq(2) + expect(snippets).to all(be_a(PersonalSnippet)) + expect(response).to have_gitlab_http_status(:ok) + end + + it 'renders pagination' do + get :index, params: { page: 2 } + + snippets = assigns(:snippets) + + expect(snippets).to be_a(::Kaminari::PaginatableWithoutCount) + expect(snippets.size).to eq(1) + expect(assigns(:snippets)).to all(be_a(PersonalSnippet)) + expect(response).to have_gitlab_http_status(:ok) end end end diff --git a/spec/javascripts/badges/components/badge_form_spec.js b/spec/frontend/badges/components/badge_form_spec.js index c7aa7fa63b1..d61bd29ca9d 100644 --- a/spec/javascripts/badges/components/badge_form_spec.js +++ b/spec/frontend/badges/components/badge_form_spec.js @@ -1,11 +1,11 @@ import Vue from 'vue'; import MockAdapter from 'axios-mock-adapter'; -import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { mountComponentWithStore } from 'helpers/vue_mount_component_helper'; import axios from '~/lib/utils/axios_utils'; import store from '~/badges/store'; import createEmptyBadge from '~/badges/empty_badge'; import BadgeForm from '~/badges/components/badge_form.vue'; -import { DUMMY_IMAGE_URL, TEST_HOST } from '../../test_constants'; +import { DUMMY_IMAGE_URL, TEST_HOST } from 'helpers/test_constants'; // avoid preview background process BadgeForm.methods.debouncedPreview = () => {}; @@ -41,7 +41,7 @@ describe('BadgeForm component', () => { describe('onCancel', () => { it('calls stopEditing', () => { - spyOn(vm, 'stopEditing'); + jest.spyOn(vm, 'stopEditing').mockImplementation(() => {}); vm.onCancel(); @@ -68,14 +68,14 @@ describe('BadgeForm component', () => { const expectInvalidInput = inputElementSelector => { const inputElement = vm.$el.querySelector(inputElementSelector); - expect(inputElement).toBeMatchedBy(':invalid'); + expect(inputElement.checkValidity()).toBe(false); const feedbackElement = vm.$el.querySelector(`${inputElementSelector} + .invalid-feedback`); expect(feedbackElement).toBeVisible(); }; - beforeEach(() => { - spyOn(vm, submitAction).and.returnValue(Promise.resolve()); + beforeEach(done => { + jest.spyOn(vm, submitAction).mockReturnValue(Promise.resolve()); store.replaceState({ ...store.state, badgeInAddForm: createEmptyBadge(), @@ -83,9 +83,14 @@ describe('BadgeForm component', () => { isSaving: false, }); - setValue(nameSelector, 'TestBadge'); - setValue(linkUrlSelector, `${TEST_HOST}/link/url`); - setValue(imageUrlSelector, `${window.location.origin}${DUMMY_IMAGE_URL}`); + Vue.nextTick() + .then(() => { + setValue(nameSelector, 'TestBadge'); + setValue(linkUrlSelector, `${TEST_HOST}/link/url`); + setValue(imageUrlSelector, `${window.location.origin}${DUMMY_IMAGE_URL}`); + }) + .then(done) + .catch(done.fail); }); it('returns immediately if imageUrl is empty', () => { @@ -131,8 +136,8 @@ describe('BadgeForm component', () => { it(`calls ${submitAction}`, () => { submitForm(); - expect(findImageUrlElement()).toBeMatchedBy(':valid'); - expect(findLinkUrlElement()).toBeMatchedBy(':valid'); + expect(findImageUrlElement().checkValidity()).toBe(true); + expect(findLinkUrlElement().checkValidity()).toBe(true); expect(vm[submitAction]).toHaveBeenCalled(); }); }; diff --git a/spec/javascripts/badges/components/badge_list_row_spec.js b/spec/frontend/badges/components/badge_list_row_spec.js index d1434737085..31f0d850857 100644 --- a/spec/javascripts/badges/components/badge_list_row_spec.js +++ b/spec/frontend/badges/components/badge_list_row_spec.js @@ -1,6 +1,5 @@ -import $ from 'jquery'; import Vue from 'vue'; -import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { mountComponentWithStore } from 'helpers/vue_mount_component_helper'; import { GROUP_BADGE, PROJECT_BADGE } from '~/badges/constants'; import store from '~/badges/store'; import BadgeListRow from '~/badges/components/badge_list_row.vue'; @@ -40,15 +39,15 @@ describe('BadgeListRow component', () => { }); it('renders the badge name', () => { - expect(vm.$el).toContainText(badge.name); + expect(vm.$el.innerText).toMatch(badge.name); }); it('renders the badge link', () => { - expect(vm.$el).toContainText(badge.linkUrl); + expect(vm.$el.innerText).toMatch(badge.linkUrl); }); it('renders the badge kind', () => { - expect(vm.$el).toContainText('Project Badge'); + expect(vm.$el.innerText).toMatch('Project Badge'); }); it('shows edit and delete buttons', () => { @@ -66,7 +65,7 @@ describe('BadgeListRow component', () => { }); it('calls editBadge when clicking then edit button', () => { - spyOn(vm, 'editBadge'); + jest.spyOn(vm, 'editBadge').mockImplementation(() => {}); const editButton = vm.$el.querySelector('.table-button-footer button:first-of-type'); editButton.click(); @@ -75,13 +74,17 @@ describe('BadgeListRow component', () => { }); it('calls updateBadgeInModal and shows modal when clicking then delete button', done => { - spyOn(vm, 'updateBadgeInModal'); - $('#delete-badge-modal').on('shown.bs.modal', () => done()); + jest.spyOn(vm, 'updateBadgeInModal').mockImplementation(() => {}); const deleteButton = vm.$el.querySelector('.table-button-footer button:last-of-type'); deleteButton.click(); - expect(vm.updateBadgeInModal).toHaveBeenCalled(); + Vue.nextTick() + .then(() => { + expect(vm.updateBadgeInModal).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); }); describe('for a group badge', () => { @@ -94,7 +97,7 @@ describe('BadgeListRow component', () => { }); it('renders the badge kind', () => { - expect(vm.$el).toContainText('Group Badge'); + expect(vm.$el.innerText).toMatch('Group Badge'); }); it('hides edit and delete buttons', () => { diff --git a/spec/javascripts/badges/components/badge_list_spec.js b/spec/frontend/badges/components/badge_list_spec.js index 3af194454e3..5ffc046eb97 100644 --- a/spec/javascripts/badges/components/badge_list_spec.js +++ b/spec/frontend/badges/components/badge_list_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { mountComponentWithStore } from 'helpers/vue_mount_component_helper'; import { GROUP_BADGE, PROJECT_BADGE } from '~/badges/constants'; import store from '~/badges/store'; import BadgeList from '~/badges/components/badge_list.vue'; @@ -22,6 +22,10 @@ describe('BadgeList component', () => { kind: PROJECT_BADGE, isLoading: false, }); + + // Can be removed once GlLoadingIcon no longer throws a warning + jest.spyOn(global.console, 'warn').mockImplementation(() => jest.fn()); + vm = mountComponentWithStore(Component, { el: '#dummy-element', store, @@ -49,7 +53,7 @@ describe('BadgeList component', () => { Vue.nextTick() .then(() => { - expect(vm.$el).toContainText('This project has no badges'); + expect(vm.$el.innerText).toMatch('This project has no badges'); }) .then(done) .catch(done.fail); @@ -82,7 +86,7 @@ describe('BadgeList component', () => { Vue.nextTick() .then(() => { - expect(vm.$el).toContainText('This group has no badges'); + expect(vm.$el.innerText).toMatch('This group has no badges'); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/badges/components/badge_settings_spec.js b/spec/frontend/badges/components/badge_settings_spec.js index 479a905661b..8c3f1ea2749 100644 --- a/spec/javascripts/badges/components/badge_settings_spec.js +++ b/spec/frontend/badges/components/badge_settings_spec.js @@ -1,6 +1,5 @@ -import $ from 'jquery'; import Vue from 'vue'; -import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { mountComponentWithStore } from 'helpers/vue_mount_component_helper'; import store from '~/badges/store'; import BadgeSettings from '~/badges/components/badge_settings.vue'; import { createDummyBadge } from '../dummy_badge'; @@ -19,6 +18,10 @@ describe('BadgeSettings component', () => { data-target="#delete-badge-modal" >Show modal</button> `); + + // Can be removed once GlLoadingIcon no longer throws a warning + jest.spyOn(global.console, 'warn').mockImplementation(() => jest.fn()); + vm = mountComponentWithStore(Component, { el: '#dummy-element', store, @@ -35,20 +38,16 @@ describe('BadgeSettings component', () => { const modal = vm.$el.querySelector('#delete-badge-modal'); const button = document.getElementById('dummy-modal-button'); - $(modal).on('shown.bs.modal', () => { - expect(modal).toContainText('Delete badge?'); - const badgeElement = modal.querySelector('img.project-badge'); - - expect(badgeElement).not.toBe(null); - expect(badgeElement.getAttribute('src')).toBe(badge.renderedImageUrl); - - done(); - }); + button.click(); Vue.nextTick() .then(() => { - button.click(); + expect(modal.innerText).toMatch('Delete badge?'); + const badgeElement = modal.querySelector('img.project-badge'); + expect(badgeElement).not.toBe(null); + expect(badgeElement.getAttribute('src')).toBe(badge.renderedImageUrl); }) + .then(done) .catch(done.fail); }); @@ -67,7 +66,7 @@ describe('BadgeSettings component', () => { expect(badgeListElement).not.toBe(null); expect(badgeListElement).toBeVisible(); - expect(badgeListElement).toContainText('Your badges'); + expect(badgeListElement.innerText).toMatch('Your badges'); }); describe('when editing', () => { @@ -103,7 +102,7 @@ describe('BadgeSettings component', () => { describe('methods', () => { describe('onSubmitModal', () => { it('triggers ', () => { - spyOn(vm, 'deleteBadge').and.callFake(() => Promise.resolve()); + jest.spyOn(vm, 'deleteBadge').mockImplementation(() => Promise.resolve()); const modal = vm.$el.querySelector('#delete-badge-modal'); const deleteButton = modal.querySelector('.btn-danger'); diff --git a/spec/javascripts/badges/components/badge_spec.js b/spec/frontend/badges/components/badge_spec.js index 14490b1bbd1..43004004fb2 100644 --- a/spec/javascripts/badges/components/badge_spec.js +++ b/spec/frontend/badges/components/badge_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import mountComponent from 'helpers/vue_mount_component_helper'; import { DUMMY_IMAGE_URL, TEST_HOST } from 'spec/test_constants'; import Badge from '~/badges/components/badge.vue'; @@ -23,9 +23,11 @@ describe('Badge component', () => { const createComponent = (props, el = null) => { vm = mountComponent(Component, props, el); const { badgeImage } = findElements(); - return new Promise(resolve => badgeImage.addEventListener('load', resolve)).then(() => - Vue.nextTick(), - ); + return new Promise(resolve => { + badgeImage.addEventListener('load', resolve); + // Manually dispatch load event as it is not triggered + badgeImage.dispatchEvent(new Event('load')); + }).then(() => Vue.nextTick()); }; afterEach(() => { @@ -111,7 +113,7 @@ describe('Badge component', () => { expect(badgeImage).toBeVisible(); expect(loadingIcon).toBeHidden(); expect(reloadButton).toBeHidden(); - expect(vm.$el.innerText).toBe(''); + expect(vm.$el.querySelector('.btn-group')).toBeHidden(); }); it('shows a loading icon when loading', done => { @@ -124,7 +126,7 @@ describe('Badge component', () => { expect(badgeImage).toBeHidden(); expect(loadingIcon).toBeVisible(); expect(reloadButton).toBeHidden(); - expect(vm.$el.innerText).toBe(''); + expect(vm.$el.querySelector('.btn-group')).toBeHidden(); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/badges/dummy_badge.js b/spec/frontend/badges/dummy_badge.js index a0dee89736e..a0dee89736e 100644 --- a/spec/javascripts/badges/dummy_badge.js +++ b/spec/frontend/badges/dummy_badge.js diff --git a/spec/javascripts/badges/store/actions_spec.js b/spec/frontend/badges/store/actions_spec.js index d92155d59b5..921c21cb55e 100644 --- a/spec/javascripts/badges/store/actions_spec.js +++ b/spec/frontend/badges/store/actions_spec.js @@ -1,6 +1,6 @@ import MockAdapter from 'axios-mock-adapter'; import { TEST_HOST } from 'spec/test_constants'; -import testAction from 'spec/helpers/vuex_action_helper'; +import testAction from 'helpers/vuex_action_helper'; import axios from '~/lib/utils/axios_utils'; import actions, { transformBackendBadge } from '~/badges/store/actions'; import mutationTypes from '~/badges/store/mutation_types'; @@ -76,7 +76,7 @@ describe('Badges store actions', () => { beforeEach(() => { endpointMock = axiosMock.onPost(dummyEndpointUrl); - dispatch = jasmine.createSpy('dispatch'); + dispatch = jest.fn(); badgeInAddForm = createDummyBadge(); state = { ...state, @@ -96,8 +96,8 @@ describe('Badges store actions', () => { }), ); - expect(dispatch.calls.allArgs()).toEqual([['requestNewBadge']]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestNewBadge']]); + dispatch.mockClear(); return [200, dummyResponse]; }); @@ -105,7 +105,7 @@ describe('Badges store actions', () => { actions .addBadge({ state, dispatch }) .then(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveNewBadge', dummyBadge]]); + expect(dispatch.mock.calls).toEqual([['receiveNewBadge', dummyBadge]]); }) .then(done) .catch(done.fail); @@ -121,8 +121,8 @@ describe('Badges store actions', () => { }), ); - expect(dispatch.calls.allArgs()).toEqual([['requestNewBadge']]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestNewBadge']]); + dispatch.mockClear(); return [500, '']; }); @@ -130,7 +130,7 @@ describe('Badges store actions', () => { .addBadge({ state, dispatch }) .then(() => done.fail('Expected Ajax call to fail!')) .catch(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveNewBadgeError']]); + expect(dispatch.mock.calls).toEqual([['receiveNewBadgeError']]); }) .then(done) .catch(done.fail); @@ -182,20 +182,20 @@ describe('Badges store actions', () => { beforeEach(() => { endpointMock = axiosMock.onDelete(`${dummyEndpointUrl}/${badgeId}`); - dispatch = jasmine.createSpy('dispatch'); + dispatch = jest.fn(); }); it('dispatches requestDeleteBadge and receiveDeleteBadge for successful response', done => { endpointMock.replyOnce(() => { - expect(dispatch.calls.allArgs()).toEqual([['requestDeleteBadge', badgeId]]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestDeleteBadge', badgeId]]); + dispatch.mockClear(); return [200, '']; }); actions .deleteBadge({ state, dispatch }, { id: badgeId }) .then(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveDeleteBadge', badgeId]]); + expect(dispatch.mock.calls).toEqual([['receiveDeleteBadge', badgeId]]); }) .then(done) .catch(done.fail); @@ -203,8 +203,8 @@ describe('Badges store actions', () => { it('dispatches requestDeleteBadge and receiveDeleteBadgeError for error response', done => { endpointMock.replyOnce(() => { - expect(dispatch.calls.allArgs()).toEqual([['requestDeleteBadge', badgeId]]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestDeleteBadge', badgeId]]); + dispatch.mockClear(); return [500, '']; }); @@ -212,7 +212,7 @@ describe('Badges store actions', () => { .deleteBadge({ state, dispatch }, { id: badgeId }) .then(() => done.fail('Expected Ajax call to fail!')) .catch(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveDeleteBadgeError', badgeId]]); + expect(dispatch.mock.calls).toEqual([['receiveDeleteBadgeError', badgeId]]); }) .then(done) .catch(done.fail); @@ -280,7 +280,7 @@ describe('Badges store actions', () => { beforeEach(() => { endpointMock = axiosMock.onGet(dummyEndpointUrl); - dispatch = jasmine.createSpy('dispatch'); + dispatch = jest.fn(); }); it('dispatches requestLoadBadges and receiveLoadBadges for successful response', done => { @@ -291,8 +291,8 @@ describe('Badges store actions', () => { createDummyBadgeResponse(), ]; endpointMock.replyOnce(() => { - expect(dispatch.calls.allArgs()).toEqual([['requestLoadBadges', dummyData]]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestLoadBadges', dummyData]]); + dispatch.mockClear(); return [200, dummyReponse]; }); @@ -301,7 +301,7 @@ describe('Badges store actions', () => { .then(() => { const badges = dummyReponse.map(transformBackendBadge); - expect(dispatch.calls.allArgs()).toEqual([['receiveLoadBadges', badges]]); + expect(dispatch.mock.calls).toEqual([['receiveLoadBadges', badges]]); }) .then(done) .catch(done.fail); @@ -310,8 +310,8 @@ describe('Badges store actions', () => { it('dispatches requestLoadBadges and receiveLoadBadgesError for error response', done => { const dummyData = 'this is just some data'; endpointMock.replyOnce(() => { - expect(dispatch.calls.allArgs()).toEqual([['requestLoadBadges', dummyData]]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestLoadBadges', dummyData]]); + dispatch.mockClear(); return [500, '']; }); @@ -319,7 +319,7 @@ describe('Badges store actions', () => { .loadBadges({ state, dispatch }, dummyData) .then(() => done.fail('Expected Ajax call to fail!')) .catch(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveLoadBadgesError']]); + expect(dispatch.mock.calls).toEqual([['receiveLoadBadgesError']]); }) .then(done) .catch(done.fail); @@ -382,11 +382,11 @@ describe('Badges store actions', () => { `image_url=${encodeURIComponent(badgeInForm.imageUrl)}`, ].join('&'); endpointMock = axiosMock.onGet(`${dummyEndpointUrl}/render?${urlParameters}`); - dispatch = jasmine.createSpy('dispatch'); + dispatch = jest.fn(); }); it('returns immediately if imageUrl is empty', done => { - spyOn(axios, 'get'); + jest.spyOn(axios, 'get').mockImplementation(() => {}); badgeInForm.imageUrl = ''; actions @@ -399,7 +399,7 @@ describe('Badges store actions', () => { }); it('returns immediately if linkUrl is empty', done => { - spyOn(axios, 'get'); + jest.spyOn(axios, 'get').mockImplementation(() => {}); badgeInForm.linkUrl = ''; actions @@ -412,19 +412,23 @@ describe('Badges store actions', () => { }); it('escapes user input', done => { - spyOn(axios, 'get').and.callFake(() => Promise.resolve({ data: createDummyBadgeResponse() })); + jest + .spyOn(axios, 'get') + .mockImplementation(() => Promise.resolve({ data: createDummyBadgeResponse() })); badgeInForm.imageUrl = '&make-sandwich=true'; badgeInForm.linkUrl = '<script>I am dangerous!</script>'; actions .renderBadge({ state, dispatch }) .then(() => { - expect(axios.get.calls.count()).toBe(1); - const url = axios.get.calls.argsFor(0)[0]; - - expect(url).toMatch(`^${dummyEndpointUrl}/render?`); - expect(url).toMatch('\\?link_url=%3Cscript%3EI%20am%20dangerous!%3C%2Fscript%3E&'); - expect(url).toMatch('&image_url=%26make-sandwich%3Dtrue$'); + expect(axios.get.mock.calls.length).toBe(1); + const url = axios.get.mock.calls[0][0]; + + expect(url).toMatch(new RegExp(`^${dummyEndpointUrl}/render?`)); + expect(url).toMatch( + new RegExp('\\?link_url=%3Cscript%3EI%20am%20dangerous!%3C%2Fscript%3E&'), + ); + expect(url).toMatch(new RegExp('&image_url=%26make-sandwich%3Dtrue$')); }) .then(done) .catch(done.fail); @@ -433,8 +437,8 @@ describe('Badges store actions', () => { it('dispatches requestRenderedBadge and receiveRenderedBadge for successful response', done => { const dummyReponse = createDummyBadgeResponse(); endpointMock.replyOnce(() => { - expect(dispatch.calls.allArgs()).toEqual([['requestRenderedBadge']]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestRenderedBadge']]); + dispatch.mockClear(); return [200, dummyReponse]; }); @@ -443,7 +447,7 @@ describe('Badges store actions', () => { .then(() => { const renderedBadge = transformBackendBadge(dummyReponse); - expect(dispatch.calls.allArgs()).toEqual([['receiveRenderedBadge', renderedBadge]]); + expect(dispatch.mock.calls).toEqual([['receiveRenderedBadge', renderedBadge]]); }) .then(done) .catch(done.fail); @@ -451,8 +455,8 @@ describe('Badges store actions', () => { it('dispatches requestRenderedBadge and receiveRenderedBadgeError for error response', done => { endpointMock.replyOnce(() => { - expect(dispatch.calls.allArgs()).toEqual([['requestRenderedBadge']]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestRenderedBadge']]); + dispatch.mockClear(); return [500, '']; }); @@ -460,7 +464,7 @@ describe('Badges store actions', () => { .renderBadge({ state, dispatch }) .then(() => done.fail('Expected Ajax call to fail!')) .catch(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveRenderedBadgeError']]); + expect(dispatch.mock.calls).toEqual([['receiveRenderedBadgeError']]); }) .then(done) .catch(done.fail); @@ -519,7 +523,7 @@ describe('Badges store actions', () => { badgeInEditForm, }; endpointMock = axiosMock.onPut(`${dummyEndpointUrl}/${badgeInEditForm.id}`); - dispatch = jasmine.createSpy('dispatch'); + dispatch = jest.fn(); }); it('dispatches requestUpdatedBadge and receiveUpdatedBadge for successful response', done => { @@ -534,8 +538,8 @@ describe('Badges store actions', () => { }), ); - expect(dispatch.calls.allArgs()).toEqual([['requestUpdatedBadge']]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestUpdatedBadge']]); + dispatch.mockClear(); return [200, dummyResponse]; }); @@ -543,7 +547,7 @@ describe('Badges store actions', () => { actions .saveBadge({ state, dispatch }) .then(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveUpdatedBadge', updatedBadge]]); + expect(dispatch.mock.calls).toEqual([['receiveUpdatedBadge', updatedBadge]]); }) .then(done) .catch(done.fail); @@ -559,8 +563,8 @@ describe('Badges store actions', () => { }), ); - expect(dispatch.calls.allArgs()).toEqual([['requestUpdatedBadge']]); - dispatch.calls.reset(); + expect(dispatch.mock.calls).toEqual([['requestUpdatedBadge']]); + dispatch.mockClear(); return [500, '']; }); @@ -568,7 +572,7 @@ describe('Badges store actions', () => { .saveBadge({ state, dispatch }) .then(() => done.fail('Expected Ajax call to fail!')) .catch(() => { - expect(dispatch.calls.allArgs()).toEqual([['receiveUpdatedBadgeError']]); + expect(dispatch.mock.calls).toEqual([['receiveUpdatedBadgeError']]); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/badges/store/mutations_spec.js b/spec/frontend/badges/store/mutations_spec.js index 8d26f83339d..8d26f83339d 100644 --- a/spec/javascripts/badges/store/mutations_spec.js +++ b/spec/frontend/badges/store/mutations_spec.js diff --git a/spec/frontend/notes/components/discussion_counter_spec.js b/spec/frontend/notes/components/discussion_counter_spec.js new file mode 100644 index 00000000000..c9375df07e8 --- /dev/null +++ b/spec/frontend/notes/components/discussion_counter_spec.js @@ -0,0 +1,91 @@ +import Vuex from 'vuex'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import notesModule from '~/notes/stores/modules'; +import DiscussionCounter from '~/notes/components/discussion_counter.vue'; +import { noteableDataMock, discussionMock, notesDataMock, userDataMock } from '../mock_data'; +import * as types from '~/notes/stores/mutation_types'; + +describe('DiscussionCounter component', () => { + let store; + let wrapper; + const localVue = createLocalVue(); + + localVue.use(Vuex); + + beforeEach(() => { + window.mrTabs = {}; + const { state, getters, mutations, actions } = notesModule(); + + store = new Vuex.Store({ + state: { + ...state, + userData: userDataMock, + }, + getters, + mutations, + actions, + }); + store.dispatch('setNoteableData', { + ...noteableDataMock, + create_issue_to_resolve_discussions_path: '/test', + }); + store.dispatch('setNotesData', notesDataMock); + }); + + afterEach(() => { + wrapper.vm.$destroy(); + wrapper = null; + }); + + describe('has no discussions', () => { + it('does not render', () => { + wrapper = shallowMount(DiscussionCounter, { store, localVue }); + + expect(wrapper.find({ ref: 'discussionCounter' }).exists()).toBe(false); + }); + }); + + describe('has no resolvable discussions', () => { + it('does not render', () => { + store.commit(types.SET_INITIAL_DISCUSSIONS, [{ ...discussionMock, resolvable: false }]); + store.dispatch('updateResolvableDiscussionsCounts'); + wrapper = shallowMount(DiscussionCounter, { store, localVue }); + + expect(wrapper.find({ ref: 'discussionCounter' }).exists()).toBe(false); + }); + }); + + describe('has resolvable discussions', () => { + const updateStore = (note = {}) => { + discussionMock.notes[0] = { ...discussionMock.notes[0], ...note }; + store.commit(types.SET_INITIAL_DISCUSSIONS, [discussionMock]); + store.dispatch('updateResolvableDiscussionsCounts'); + }; + + afterEach(() => { + delete discussionMock.notes[0].resolvable; + delete discussionMock.notes[0].resolved; + }); + + it('renders', () => { + updateStore(); + wrapper = shallowMount(DiscussionCounter, { store, localVue }); + + expect(wrapper.find({ ref: 'discussionCounter' }).exists()).toBe(true); + }); + + it.each` + title | resolved | hasNextBtn | isActive | icon | groupLength + ${'hasNextButton'} | ${false} | ${true} | ${false} | ${'check-circle'} | ${2} + ${'allResolved'} | ${true} | ${false} | ${true} | ${'check-circle-filled'} | ${0} + `('renders correctly if $title', ({ resolved, hasNextBtn, isActive, icon, groupLength }) => { + updateStore({ resolvable: true, resolved }); + wrapper = shallowMount(DiscussionCounter, { store, localVue }); + + expect(wrapper.find(`.has-next-btn`).exists()).toBe(hasNextBtn); + expect(wrapper.find(`.is-active`).exists()).toBe(isActive); + expect(wrapper.find({ name: icon }).exists()).toBe(true); + expect(wrapper.findAll('[role="group"').length).toBe(groupLength); + }); + }); +}); diff --git a/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js b/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js index a00dd445c4f..2ff9dbc5c19 100644 --- a/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js +++ b/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js @@ -3,9 +3,12 @@ import JumpToNextDiscussionButton from '~/notes/components/discussion_jump_to_ne describe('JumpToNextDiscussionButton', () => { let wrapper; + const fromDiscussionId = 'abc123'; beforeEach(() => { - wrapper = shallowMount(JumpToNextDiscussionButton); + wrapper = shallowMount(JumpToNextDiscussionButton, { + propsData: { fromDiscussionId }, + }); }); afterEach(() => { @@ -15,4 +18,11 @@ describe('JumpToNextDiscussionButton', () => { it('matches the snapshot', () => { expect(wrapper.vm.$el).toMatchSnapshot(); }); + + it('calls jumpToNextRelativeDiscussion when clicked', () => { + const jumpToNextRelativeDiscussion = jest.fn(); + wrapper.setMethods({ jumpToNextRelativeDiscussion }); + wrapper.find({ ref: 'button' }).trigger('click'); + expect(jumpToNextRelativeDiscussion).toHaveBeenCalledWith(fromDiscussionId); + }); }); diff --git a/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js b/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js index 74e827784ec..e932133b869 100644 --- a/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js +++ b/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js @@ -1,84 +1,53 @@ /* global Mousetrap */ import 'mousetrap'; import { shallowMount, createLocalVue } from '@vue/test-utils'; -import Vuex from 'vuex'; import DiscussionKeyboardNavigator from '~/notes/components/discussion_keyboard_navigator.vue'; -import notesModule from '~/notes/stores/modules'; - -const localVue = createLocalVue(); -localVue.use(Vuex); - -const NEXT_ID = 'abc123'; -const PREV_ID = 'def456'; -const NEXT_DIFF_ID = 'abc123_diff'; -const PREV_DIFF_ID = 'def456_diff'; describe('notes/components/discussion_keyboard_navigator', () => { - let storeOptions; - let wrapper; - let store; + const localVue = createLocalVue(); - const createComponent = (options = {}) => { - store = new Vuex.Store(storeOptions); + let wrapper; + let jumpToNextDiscussion; + let jumpToPreviousDiscussion; + const createComponent = () => { wrapper = shallowMount(DiscussionKeyboardNavigator, { - localVue, - store, - ...options, + mixins: [ + localVue.extend({ + methods: { + jumpToNextDiscussion, + jumpToPreviousDiscussion, + }, + }), + ], }); - - wrapper.vm.jumpToDiscussion = jest.fn(); }; beforeEach(() => { - const notes = notesModule(); - - notes.getters.nextUnresolvedDiscussionId = () => (currId, isDiff) => - isDiff ? NEXT_DIFF_ID : NEXT_ID; - notes.getters.previousUnresolvedDiscussionId = () => (currId, isDiff) => - isDiff ? PREV_DIFF_ID : PREV_ID; - notes.getters.getDiscussion = () => id => ({ id }); - - storeOptions = { - modules: { - notes, - }, - }; + jumpToNextDiscussion = jest.fn(); + jumpToPreviousDiscussion = jest.fn(); }); afterEach(() => { wrapper.destroy(); - storeOptions = null; - store = null; + wrapper = null; }); - describe.each` - currentAction | expectedNextId | expectedPrevId - ${'diffs'} | ${NEXT_DIFF_ID} | ${PREV_DIFF_ID} - ${'show'} | ${NEXT_ID} | ${PREV_ID} - `('when isDiffView is $isDiffView', ({ currentAction, expectedNextId, expectedPrevId }) => { + describe('on mount', () => { beforeEach(() => { - window.mrTabs = { currentAction }; createComponent(); }); - afterEach(() => delete window.mrTabs); it('calls jumpToNextDiscussion when pressing `n`', () => { Mousetrap.trigger('n'); - expect(wrapper.vm.jumpToDiscussion).toHaveBeenCalledWith( - expect.objectContaining({ id: expectedNextId }), - ); - expect(wrapper.vm.currentDiscussionId).toEqual(expectedNextId); + expect(jumpToNextDiscussion).toHaveBeenCalled(); }); it('calls jumpToPreviousDiscussion when pressing `p`', () => { Mousetrap.trigger('p'); - expect(wrapper.vm.jumpToDiscussion).toHaveBeenCalledWith( - expect.objectContaining({ id: expectedPrevId }), - ); - expect(wrapper.vm.currentDiscussionId).toEqual(expectedPrevId); + expect(jumpToPreviousDiscussion).toHaveBeenCalled(); }); }); @@ -99,13 +68,13 @@ describe('notes/components/discussion_keyboard_navigator', () => { it('does not call jumpToNextDiscussion when pressing `n`', () => { Mousetrap.trigger('n'); - expect(wrapper.vm.jumpToDiscussion).not.toHaveBeenCalled(); + expect(jumpToNextDiscussion).not.toHaveBeenCalled(); }); it('does not call jumpToNextDiscussion when pressing `p`', () => { Mousetrap.trigger('p'); - expect(wrapper.vm.jumpToDiscussion).not.toHaveBeenCalled(); + expect(jumpToPreviousDiscussion).not.toHaveBeenCalled(); }); }); }); diff --git a/spec/frontend/notes/mixins/discussion_navigation_spec.js b/spec/frontend/notes/mixins/discussion_navigation_spec.js new file mode 100644 index 00000000000..4e5325b8bc3 --- /dev/null +++ b/spec/frontend/notes/mixins/discussion_navigation_spec.js @@ -0,0 +1,178 @@ +import Vuex from 'vuex'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import * as utils from '~/lib/utils/common_utils'; +import discussionNavigation from '~/notes/mixins/discussion_navigation'; +import eventHub from '~/notes/event_hub'; +import notesModule from '~/notes/stores/modules'; +import { setHTMLFixture } from 'helpers/fixtures'; + +const discussion = (id, index) => ({ + id, + resolvable: index % 2 === 0, + active: true, + notes: [{}], + diff_discussion: true, +}); +const createDiscussions = () => [...'abcde'].map(discussion); +const createComponent = () => ({ + mixins: [discussionNavigation], + render() { + return this.$slots.default; + }, +}); + +describe('Discussion navigation mixin', () => { + const localVue = createLocalVue(); + localVue.use(Vuex); + + let wrapper; + let store; + let expandDiscussion; + + beforeEach(() => { + setHTMLFixture( + [...'abcde'] + .map( + id => + `<ul class="notes" data-discussion-id="${id}"></ul> + <div class="discussion" data-discussion-id="${id}"></div>`, + ) + .join(''), + ); + + jest.spyOn(utils, 'scrollToElement'); + + expandDiscussion = jest.fn(); + const { actions, ...notesRest } = notesModule(); + store = new Vuex.Store({ + modules: { + notes: { + ...notesRest, + actions: { ...actions, expandDiscussion }, + }, + }, + }); + store.state.notes.discussions = createDiscussions(); + + wrapper = shallowMount(createComponent(), { store, localVue }); + }); + + afterEach(() => { + wrapper.vm.$destroy(); + jest.clearAllMocks(); + }); + + const findDiscussion = (selector, id) => + document.querySelector(`${selector}[data-discussion-id="${id}"]`); + + describe('cycle through discussions', () => { + beforeEach(() => { + // eslint-disable-next-line new-cap + window.mrTabs = { eventHub: new localVue(), tabShown: jest.fn() }; + }); + + describe.each` + fn | args | currentId | expected + ${'jumpToNextDiscussion'} | ${[]} | ${null} | ${'a'} + ${'jumpToNextDiscussion'} | ${[]} | ${'a'} | ${'c'} + ${'jumpToNextDiscussion'} | ${[]} | ${'e'} | ${'a'} + ${'jumpToPreviousDiscussion'} | ${[]} | ${null} | ${'e'} + ${'jumpToPreviousDiscussion'} | ${[]} | ${'e'} | ${'c'} + ${'jumpToPreviousDiscussion'} | ${[]} | ${'c'} | ${'a'} + ${'jumpToNextRelativeDiscussion'} | ${[null]} | ${null} | ${'a'} + ${'jumpToNextRelativeDiscussion'} | ${['a']} | ${null} | ${'c'} + ${'jumpToNextRelativeDiscussion'} | ${['e']} | ${'c'} | ${'a'} + `('$fn (args = $args, currentId = $currentId)', ({ fn, args, currentId, expected }) => { + beforeEach(() => { + store.state.notes.currentDiscussionId = currentId; + }); + + describe('on `show` active tab', () => { + beforeEach(() => { + window.mrTabs.currentAction = 'show'; + wrapper.vm[fn](...args); + }); + + it('sets current discussion', () => { + expect(store.state.notes.currentDiscussionId).toEqual(expected); + }); + + it('expands discussion', () => { + expect(expandDiscussion).toHaveBeenCalled(); + }); + + it('scrolls to element', () => { + expect(utils.scrollToElement).toHaveBeenCalledWith( + findDiscussion('div.discussion', expected), + ); + }); + }); + + describe('on `diffs` active tab', () => { + beforeEach(() => { + window.mrTabs.currentAction = 'diffs'; + wrapper.vm[fn](...args); + }); + + it('sets current discussion', () => { + expect(store.state.notes.currentDiscussionId).toEqual(expected); + }); + + it('expands discussion', () => { + expect(expandDiscussion).toHaveBeenCalled(); + }); + + it('scrolls when scrollToDiscussion is emitted', () => { + expect(utils.scrollToElement).not.toHaveBeenCalled(); + + eventHub.$emit('scrollToDiscussion'); + + expect(utils.scrollToElement).toHaveBeenCalledWith(findDiscussion('ul.notes', expected)); + }); + }); + + describe('on `other` active tab', () => { + beforeEach(() => { + window.mrTabs.currentAction = 'other'; + wrapper.vm[fn](...args); + }); + + it('sets current discussion', () => { + expect(store.state.notes.currentDiscussionId).toEqual(expected); + }); + + it('does not expand discussion yet', () => { + expect(expandDiscussion).not.toHaveBeenCalled(); + }); + + it('shows mrTabs', () => { + expect(window.mrTabs.tabShown).toHaveBeenCalledWith('show'); + }); + + describe('when tab is changed', () => { + beforeEach(() => { + window.mrTabs.eventHub.$emit('MergeRequestTabChange'); + + jest.runAllTimers(); + }); + + it('expands discussion', () => { + expect(expandDiscussion).toHaveBeenCalledWith( + expect.anything(), + { + discussionId: expected, + }, + undefined, + ); + }); + + it('scrolls to discussion', () => { + expect(utils.scrollToElement).toHaveBeenCalledWith( + findDiscussion('div.discussion', expected), + ); + }); + }); + }); + }); + }); +}); diff --git a/spec/javascripts/notes/components/discussion_counter_spec.js b/spec/javascripts/notes/components/discussion_counter_spec.js deleted file mode 100644 index 9c7aed43a3b..00000000000 --- a/spec/javascripts/notes/components/discussion_counter_spec.js +++ /dev/null @@ -1,90 +0,0 @@ -import Vue from 'vue'; -import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import createStore from '~/notes/stores'; -import DiscussionCounter from '~/notes/components/discussion_counter.vue'; -import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data'; - -describe('DiscussionCounter component', () => { - let store; - let vm; - const notes = { currentDiscussionId: null }; - - beforeEach(() => { - window.mrTabs = {}; - - const Component = Vue.extend(DiscussionCounter); - - store = createStore(); - store.dispatch('setNoteableData', noteableDataMock); - store.dispatch('setNotesData', notesDataMock); - - vm = createComponentWithStore(Component, store); - }); - - afterEach(() => { - vm.$destroy(); - }); - - describe('methods', () => { - describe('jumpToNextDiscussion', () => { - it('expands unresolved discussion', () => { - window.mrTabs.currentAction = 'show'; - - spyOn(vm, 'expandDiscussion').and.stub(); - const discussions = [ - { - ...discussionMock, - id: discussionMock.id, - notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }], - resolved: true, - }, - { - ...discussionMock, - id: discussionMock.id + 1, - notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }], - resolved: false, - }, - ]; - const firstDiscussionId = discussionMock.id + 1; - store.replaceState({ - ...store.state, - discussions, - notes, - }); - vm.jumpToNextDiscussion(); - - expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: firstDiscussionId }); - }); - - it('jumps to next unresolved discussion from diff tab if all diff discussions are resolved', () => { - window.mrTabs.currentAction = 'diff'; - spyOn(vm, 'switchToDiscussionsTabAndJumpTo').and.stub(); - - const unresolvedId = discussionMock.id + 1; - const discussions = [ - { - ...discussionMock, - id: discussionMock.id, - diff_discussion: true, - notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }], - resolved: true, - }, - { - ...discussionMock, - id: unresolvedId, - notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }], - resolved: false, - }, - ]; - store.replaceState({ - ...store.state, - discussions, - notes, - }); - vm.jumpToNextDiscussion(); - - expect(vm.switchToDiscussionsTabAndJumpTo).toHaveBeenCalledWith(unresolvedId); - }); - }); - }); -}); diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 1d652f703b8..7d98f8a0c3e 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -271,6 +271,7 @@ MergeRequest::Metrics: - diff_size - modified_paths_size - commits_count +- first_approved_at Ci::Pipeline: - id - project_id diff --git a/spec/lib/gitlab/job_waiter_spec.rb b/spec/lib/gitlab/job_waiter_spec.rb index efa7fd4b975..da6a6a9149b 100644 --- a/spec/lib/gitlab/job_waiter_spec.rb +++ b/spec/lib/gitlab/job_waiter_spec.rb @@ -37,5 +37,40 @@ describe Gitlab::JobWaiter do expect(result).to contain_exactly('a') end + + context 'when a label is provided' do + let(:waiter) { described_class.new(2, worker_label: 'Foo') } + let(:started_total) { double(:started_total) } + let(:timeouts_total) { double(:timeouts_total) } + + before do + allow(Gitlab::Metrics).to receive(:counter) + .with(described_class::STARTED_METRIC, anything) + .and_return(started_total) + + allow(Gitlab::Metrics).to receive(:counter) + .with(described_class::TIMEOUTS_METRIC, anything) + .and_return(timeouts_total) + end + + it 'increments just job_waiter_started_total when all jobs complete' do + expect(started_total).to receive(:increment).with(worker: 'Foo') + expect(timeouts_total).not_to receive(:increment) + + described_class.notify(waiter.key, 'a') + described_class.notify(waiter.key, 'b') + + result = nil + expect { Timeout.timeout(1) { result = waiter.wait(2) } }.not_to raise_error + end + + it 'increments job_waiter_started_total and job_waiter_timeouts_total when it times out' do + expect(started_total).to receive(:increment).with(worker: 'Foo') + expect(timeouts_total).to receive(:increment).with(worker: 'Foo') + + result = nil + expect { Timeout.timeout(2) { result = waiter.wait(1) } }.not_to raise_error + end + end end end diff --git a/spec/lib/gitlab/tracing_spec.rb b/spec/lib/gitlab/tracing_spec.rb deleted file mode 100644 index e913bb600ec..00000000000 --- a/spec/lib/gitlab/tracing_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' -require 'rspec-parameterized' - -describe Gitlab::Tracing do - using RSpec::Parameterized::TableSyntax - - describe '.enabled?' do - where(:connection_string, :enabled_state) do - nil | false - "" | false - "opentracing://jaeger" | true - end - - with_them do - it 'returns the correct state for .enabled?' do - expect(described_class).to receive(:connection_string).and_return(connection_string) - - expect(described_class.enabled?).to eq(enabled_state) - end - end - end - - describe '.tracing_url_enabled?' do - where(:enabled?, :tracing_url_template, :tracing_url_enabled_state) do - false | nil | false - false | "" | false - false | "http://localhost" | false - true | nil | false - true | "" | false - true | "http://localhost" | true - end - - with_them do - it 'returns the correct state for .tracing_url_enabled?' do - expect(described_class).to receive(:enabled?).and_return(enabled?) - allow(described_class).to receive(:tracing_url_template).and_return(tracing_url_template) - - expect(described_class.tracing_url_enabled?).to eq(tracing_url_enabled_state) - end - end - end - - describe '.tracing_url' do - where(:tracing_url_enabled?, :tracing_url_template, :correlation_id, :process_name, :tracing_url) do - false | "https://localhost" | "123" | "web" | nil - true | "https://localhost" | "123" | "web" | "https://localhost" - true | "https://localhost?service={{ service }}" | "123" | "web" | "https://localhost?service=web" - true | "https://localhost?c={{ correlation_id }}" | "123" | "web" | "https://localhost?c=123" - true | "https://localhost?c={{ correlation_id }}&s={{ service }}" | "123" | "web" | "https://localhost?c=123&s=web" - true | "https://localhost?c={{ correlation_id }}" | nil | "web" | "https://localhost?c=" - true | "https://localhost?c={{ correlation_id }}&s=%22{{ service }}%22" | "123" | "web" | "https://localhost?c=123&s=%22web%22" - true | "https://localhost?c={{correlation_id}}&s={{service}}" | "123" | "web" | "https://localhost?c=123&s=web" - true | "https://localhost?c={{correlation_id }}&s={{ service}}" | "123" | "web" | "https://localhost?c=123&s=web" - end - - with_them do - it 'returns the correct state for .tracing_url' do - expect(described_class).to receive(:tracing_url_enabled?).and_return(tracing_url_enabled?) - allow(described_class).to receive(:tracing_url_template).and_return(tracing_url_template) - allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return(correlation_id) - allow(Gitlab).to receive(:process_name).and_return(process_name) - - expect(described_class.tracing_url).to eq(tracing_url) - end - end - end -end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 4bfb5771bb8..37219365ecf 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2509,6 +2509,64 @@ describe Ci::Build do end end + describe 'CHANGED_PAGES variables' do + let(:route_map_yaml) do + <<~ROUTEMAP + - source: 'bar/branch-test.txt' + public: '/bar/branches' + ROUTEMAP + end + + before do + allow_any_instance_of(Project) + .to receive(:route_map_for).with(/.+/) + .and_return(Gitlab::RouteMap.new(route_map_yaml)) + end + + context 'with a deployment environment and a merge request' do + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:environment) { create(:environment, project: merge_request.project, name: "foo-#{project.default_branch}") } + let(:build) { create(:ci_build, pipeline: pipeline, environment: environment.name) } + + it 'populates CI_MERGE_REQUEST_CHANGED_PAGES_* variables' do + expect(subject).to include( + { key: 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS', value: '/bar/branches', public: true, masked: false }, + { key: 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS', value: File.join(environment.external_url, '/bar/branches'), public: true, masked: false } + ) + end + + context 'with a deployment environment and no merge request' do + let(:environment) { create(:environment, project: project, name: "foo-#{project.default_branch}") } + let(:build) { create(:ci_build, pipeline: pipeline, environment: environment.name) } + + it 'does not append CHANGED_PAGES variables' do + ci_variables = subject.select { |var| var[:key] =~ /MERGE_REQUEST_CHANGED_PAGES/ } + + expect(ci_variables).to be_empty + end + end + + context 'with no deployment environment and a present merge request' do + let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline, source_project: project, target_project: project) } + let(:build) { create(:ci_build, pipeline: merge_request.all_pipelines.take) } + + it 'does not append CHANGED_PAGES variables' do + ci_variables = subject.select { |var| var[:key] =~ /MERGE_REQUEST_CHANGED_PAGES/ } + + expect(ci_variables).to be_empty + end + end + + context 'with no deployment environment and no merge request' do + it 'does not append CHANGED_PAGES variables' do + ci_variables = subject.select { |var| var[:key] =~ /MERGE_REQUEST_CHANGED_PAGES/ } + + expect(ci_variables).to be_empty + end + end + end + end + context 'when build has user' do let(:user_variables) do [ diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index 0f2c6928820..10283b54796 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -51,8 +51,10 @@ describe EnvironmentStatus do # - source: /files\/(.+)/ # public: '\1' describe '#changes' do + subject { environment_status.changes } + it 'contains only added and modified public pages' do - expect(environment_status.changes).to contain_exactly( + expect(subject).to contain_exactly( { path: 'ruby-style-guide.html', external_url: "#{environment.external_url}/ruby-style-guide.html" @@ -64,6 +66,18 @@ describe EnvironmentStatus do end end + describe '#changed_paths' do + subject { environment_status.changed_urls } + + it { is_expected.to contain_exactly("#{environment.external_url}/ruby-style-guide.html", "#{environment.external_url}/html/page.html") } + end + + describe '#changed_urls' do + subject { environment_status.changed_paths } + + it { is_expected.to contain_exactly('ruby-style-guide.html', 'html/page.html') } + end + describe '.for_merge_request' do let(:admin) { create(:admin) } let!(:pipeline) { create(:ci_pipeline, sha: sha, merge_requests_as_head_pipeline: [merge_request]) } diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 7b24ca5eb23..33459767302 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -643,4 +643,12 @@ describe PagesDomain do end end end + + describe '.find_by_domain_case_insensitive' do + it 'lookup is case-insensitive' do + pages_domain = create(:pages_domain, domain: "Pages.IO") + + expect(PagesDomain.find_by_domain_case_insensitive('pages.io')).to eq(pages_domain) + end + end end diff --git a/spec/models/project_services/chat_notification_service_spec.rb b/spec/models/project_services/chat_notification_service_spec.rb index 45ea4cd74ed..64c7a9b230d 100644 --- a/spec/models/project_services/chat_notification_service_spec.rb +++ b/spec/models/project_services/chat_notification_service_spec.rb @@ -74,5 +74,28 @@ describe ChatNotificationService do chat_service.execute(data) end end + + shared_examples 'with channel specified' do |channel, expected_channels| + before do + allow(chat_service).to receive(:push_channel).and_return(channel) + end + + it 'notifies all channels' do + expect(chat_service).to receive(:notify).with(any_args, hash_including(channel: expected_channels)).and_return(true) + expect(chat_service.execute(data)).to be(true) + end + end + + context 'with single channel specified' do + it_behaves_like 'with channel specified', 'slack-integration', ['slack-integration'] + end + + context 'with multiple channel names specified' do + it_behaves_like 'with channel specified', 'slack-integration,#slack-test', ['slack-integration', '#slack-test'] + end + + context 'with multiple channel names with spaces specified' do + it_behaves_like 'with channel specified', 'slack-integration, #slack-test, @UDLP91W0A', ['slack-integration', '#slack-test', '@UDLP91W0A'] + end end end diff --git a/spec/requests/api/internal/pages_spec.rb b/spec/requests/api/internal/pages_spec.rb index 99cb2bfe221..44f7115f6a8 100644 --- a/spec/requests/api/internal/pages_spec.rb +++ b/spec/requests/api/internal/pages_spec.rb @@ -141,21 +141,29 @@ describe API::Internal::Pages do context 'custom domain' do let(:namespace) { create(:namespace, name: 'gitlab-org') } let(:project) { create(:project, namespace: namespace, name: 'gitlab-ce') } - let!(:pages_domain) { create(:pages_domain, domain: 'pages.gitlab.io', project: project) } + let!(:pages_domain) { create(:pages_domain, domain: 'pages.io', project: project) } context 'when there are no pages deployed for the related project' do it 'responds with 204 No Content' do - query_host('pages.gitlab.io') + query_host('pages.io') expect(response).to have_gitlab_http_status(:no_content) end end context 'when there are pages deployed for the related project' do + it 'domain lookup is case insensitive' do + deploy_pages(project) + + query_host('Pages.IO') + + expect(response).to have_gitlab_http_status(:ok) + end + it 'responds with the correct domain configuration' do deploy_pages(project) - query_host('pages.gitlab.io') + query_host('pages.io') expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('internal/pages/virtual_domain') diff --git a/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb index 2b68e7bfa82..24ff57c8517 100644 --- a/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb @@ -151,22 +151,14 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| it 'uses the username as an option for slack when configured' do allow(chat_service).to receive(:username).and_return(username) - expect(Slack::Notifier).to receive(:new) - .with(webhook_url, username: username, http_client: SlackService::Notifier::HTTPClient) - .and_return( - double(:slack_service).as_null_object - ) + expect(Slack::Messenger).to execute_with_options(username: username) chat_service.execute(data) end it 'uses the channel as an option when it is configured' do allow(chat_service).to receive(:channel).and_return(channel) - expect(Slack::Notifier).to receive(:new) - .with(webhook_url, channel: channel, http_client: SlackService::Notifier::HTTPClient) - .and_return( - double(:slack_service).as_null_object - ) + expect(Slack::Messenger).to execute_with_options(channel: [channel]) chat_service.execute(data) end @@ -174,11 +166,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| it "uses the right channel for push event" do chat_service.update(push_channel: "random") - expect(Slack::Notifier).to receive(:new) - .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient) - .and_return( - double(:slack_service).as_null_object - ) + expect(Slack::Messenger).to execute_with_options(channel: ['random']) chat_service.execute(data) end @@ -186,11 +174,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| it "uses the right channel for merge request event" do chat_service.update(merge_request_channel: "random") - expect(Slack::Notifier).to receive(:new) - .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient) - .and_return( - double(:slack_service).as_null_object - ) + expect(Slack::Messenger).to execute_with_options(channel: ['random']) chat_service.execute(@merge_sample_data) end @@ -198,11 +182,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| it "uses the right channel for issue event" do chat_service.update(issue_channel: "random") - expect(Slack::Notifier).to receive(:new) - .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient) - .and_return( - double(:slack_service).as_null_object - ) + expect(Slack::Messenger).to execute_with_options(channel: ['random']) chat_service.execute(@issues_sample_data) end @@ -213,7 +193,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| it "uses confidential issue channel" do chat_service.update(confidential_issue_channel: 'confidential') - expect(Slack::Notifier).to execute_with_options(channel: 'confidential') + expect(Slack::Messenger).to execute_with_options(channel: ['confidential']) chat_service.execute(@issues_sample_data) end @@ -221,7 +201,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| it 'falls back to issue channel' do chat_service.update(issue_channel: 'fallback_channel') - expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel') + expect(Slack::Messenger).to execute_with_options(channel: ['fallback_channel']) chat_service.execute(@issues_sample_data) end @@ -230,11 +210,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| it "uses the right channel for wiki event" do chat_service.update(wiki_page_channel: "random") - expect(Slack::Notifier).to receive(:new) - .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient) - .and_return( - double(:slack_service).as_null_object - ) + expect(Slack::Messenger).to execute_with_options(channel: ['random']) chat_service.execute(@wiki_page_sample_data) end @@ -249,11 +225,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| note_data = Gitlab::DataBuilder::Note.build(issue_note, user) - expect(Slack::Notifier).to receive(:new) - .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient) - .and_return( - double(:slack_service).as_null_object - ) + expect(Slack::Messenger).to execute_with_options(channel: ['random']) chat_service.execute(note_data) end @@ -268,7 +240,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| note_data = Gitlab::DataBuilder::Note.build(issue_note, user) - expect(Slack::Notifier).to execute_with_options(channel: 'confidential') + expect(Slack::Messenger).to execute_with_options(channel: ['confidential']) chat_service.execute(note_data) end @@ -278,7 +250,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| note_data = Gitlab::DataBuilder::Note.build(issue_note, user) - expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel') + expect(Slack::Messenger).to execute_with_options(channel: ['fallback_channel']) chat_service.execute(note_data) end |