Add latest changes from gitlab-org/gitlab@master

Gemfile

@@ -249,7 +249,7 @@ gem 'asana', '~> 0.9'
 gem 'ruby-fogbugz', '~> 0.2.1'
 
 # Kubernetes integration
-gem 'kubeclient', '~> 4.4.0'
+gem 'kubeclient', '~> 4.6.0'
 
 # Sanitize user input
 gem 'sanitize', '~> 4.6'

Gemfile.lock

@@ -286,6 +286,9 @@ GEM
     fast_gettext (1.6.0)
     ffaker (2.10.0)
     ffi (1.11.3)
+    ffi-compiler (1.0.1)
+      ffi (>= 1.0.0)
+      rake
     flipper (0.17.1)
     flipper-active_record (0.17.1)
       activerecord (>= 4.2, < 7)
@@ -493,15 +496,16 @@ GEM
     html2text (0.2.0)
       nokogiri (~> 1.6)
     htmlentities (4.3.4)
-    http (3.3.0)
+    http (4.2.0)
       addressable (~> 2.3)
       http-cookie (~> 1.0)
       http-form_data (~> 2.0)
-      http_parser.rb (~> 0.6.0)
+      http-parser (~> 1.2.0)
     http-cookie (1.0.3)
       domain_name (~> 0.5)
     http-form_data (2.1.1)
-    http_parser.rb (0.6.0)
+    http-parser (1.2.1)
+      ffi-compiler (>= 1.0, < 2.0)
     httparty (0.16.4)
       mime-types (~> 3.0)
       multi_xml (>= 0.5.2)
@@ -557,8 +561,8 @@ GEM
     kramdown (2.1.0)
     kramdown-parser-gfm (1.1.0)
       kramdown (~> 2.0)
-    kubeclient (4.4.0)
-      http (~> 3.0)
+    kubeclient (4.6.0)
+      http (>= 3.0, < 5.0)
       recursive-open-struct (~> 1.0, >= 1.0.4)
       rest-client (~> 2.0)
     launchy (2.4.3)
@@ -1247,7 +1251,7 @@ DEPENDENCIES
   jwt (~> 2.1.0)
   kaminari (~> 1.0)
   knapsack (~> 1.17)
-  kubeclient (~> 4.4.0)
+  kubeclient (~> 4.6.0)
   letter_opener_web (~> 1.3.4)
   license_finder (~> 5.4)
   licensee (~> 8.9)

app/assets/javascripts/pages/admin/broadcast_messages/broadcast_message.js

@@ -6,21 +6,31 @@ import { __ } from '~/locale';
 import { textColorForBackground } from '~/lib/utils/color_utils';
 
 export default () => {
-  const $broadcastMessageColor = $('input#broadcast_message_color');
-  const $broadcastMessagePreview = $('div.broadcast-message-preview');
+  const $broadcastMessageColor = $('.js-broadcast-message-color');
+  const $broadcastMessageType = $('.js-broadcast-message-type');
+  const $broadcastBannerMessagePreview = $('.js-broadcast-banner-message-preview');
+  const $broadcastMessage = $('.js-broadcast-message-message');
+  const previewPath = $broadcastMessage.data('previewPath');
+  const $jsBroadcastMessagePreview = $('.js-broadcast-message-preview');
+
   $broadcastMessageColor.on('input', function onMessageColorInput() {
     const previewColor = $(this).val();
-    $broadcastMessagePreview.css('background-color', previewColor);
+    $broadcastBannerMessagePreview.css('background-color', previewColor);
   });
 
   $('input#broadcast_message_font').on('input', function onMessageFontInput() {
     const previewColor = $(this).val();
-    $broadcastMessagePreview.css('color', previewColor);
+    $broadcastBannerMessagePreview.css('color', previewColor);
   });
 
-  const $broadcastMessage = $('textarea#broadcast_message_message');
-  const previewPath = $broadcastMessage.data('previewPath');
-  const $jsBroadcastMessagePreview = $('.js-broadcast-message-preview');
+  $broadcastMessageType.on('change', () => {
+    const $broadcastMessageColorFormGroup = $('.js-broadcast-message-background-color-form-group');
+    const $broadcastNotificationMessagePreview = $('.js-broadcast-notification-message-preview');
+
+    $broadcastMessageColorFormGroup.toggleClass('hidden');
+    $broadcastBannerMessagePreview.toggleClass('hidden');
+    $broadcastNotificationMessagePreview.toggleClass('hidden');
+  });
 
   $broadcastMessage.on(
     'input',
@@ -58,7 +68,7 @@ export default () => {
 
     $('.label-color-preview').css(selectedColorStyle);
 
-    return $broadcastMessagePreview.css(selectedColorStyle);
+    return $jsBroadcastMessagePreview.css(selectedColorStyle);
   };
 
   const setSuggestedColor = e => {
@@ -67,7 +77,10 @@ export default () => {
       .val(color)
       // Notify the form, that color has changed
       .trigger('input');
-    updateColorPreview();
+    // Only banner supports colors
+    if ($broadcastMessageType === 'banner') {
+      updateColorPreview();
+    }
     return e.preventDefault();
   };
 

app/assets/stylesheets/framework/broadcast_messages.scss

 .broadcast-message {
-  @extend .alert-warning;
-  padding: 10px;
-  text-align: center;
+  padding: $gl-padding-8;
 
   div,
   p {
@@ -15,9 +13,29 @@
   }
 }
 
-.broadcast-message-preview {
+.broadcast-banner-message {
+  @extend .broadcast-message;
+  @extend .alert-warning;
+  text-align: center;
+}
+
+.broadcast-notification-message {
   @extend .broadcast-message;
-  margin-bottom: 20px;
+
+  position: fixed;
+  bottom: $gl-padding;
+  right: $gl-padding;
+  max-width: 300px;
+  width: auto;
+  background: $white-light;
+  border: 1px solid $gray-200;
+  box-shadow: 0 1px 2px 0 rgba($black, 0.1);
+  border-radius: $border-radius-default;
+  z-index: 999;
+
+  &.preview {
+    position: static;
+  }
 }
 
 .toggle-colors {

app/assets/stylesheets/framework/job_log.scss

@@ -22,6 +22,7 @@
   min-width: $job-line-number-width;
   margin-left: -$job-line-number-margin;
   padding-right: 1em;
+  user-select: none;
 
   &:hover,
   &:active,

app/controllers/projects/git_http_controller.rb

@@ -75,6 +75,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def enqueue_fetch_statistics_update
+    return if Gitlab::Database.read_only?
     return if repo_type.wiki?
     return unless project&.daily_statistics_enabled?
 

app/helpers/broadcast_messages_helper.rb

 # frozen_string_literal: true
 
 module BroadcastMessagesHelper
-  def current_broadcast_messages
-    BroadcastMessage.current(request.path)
+  def current_broadcast_banner_messages
+    BroadcastMessage.current_banner_messages(request.path)
   end
 
-  def broadcast_message(message)
+  def current_broadcast_notification_message
+    BroadcastMessage.current_notification_messages(request.path).last
+  end
+
+  def broadcast_message(message, opts = {})
     return unless message.present?
 
-    content_tag :div, dir: 'auto', class: 'broadcast-message', style: broadcast_message_style(message) do
-      sprite_icon('bullhorn', size: 16, css_class: 'vertical-align-text-top mr-2') << ' ' << render_broadcast_message(message)
+    classes = "broadcast-#{message.broadcast_type}-message #{opts[:preview] && 'preview'}"
+
+    content_tag :div, dir: 'auto', class: classes, style: broadcast_message_style(message) do
+      concat sprite_icon('bullhorn', size: 16, css_class: 'vertical-align-text-top')
+      concat ' '
+      concat render_broadcast_message(message)
     end
   end
 
   def broadcast_message_style(broadcast_message)
+    return '' if broadcast_message.notification?
+
     style = []
 
     if broadcast_message.color.present?
@@ -40,4 +50,8 @@ module BroadcastMessagesHelper
   def render_broadcast_message(broadcast_message)
     Banzai.render_field(broadcast_message, :message).html_safe
   end
+
+  def broadcast_type_options
+    BroadcastMessage.broadcast_types.keys.map { |w| [w.humanize, w] }
+  end
 end

app/helpers/user_callouts_helper.rb

@@ -27,7 +27,7 @@ module UserCalloutsHelper
   end
 
   def show_tabs_feature_highlight?
-    !user_dismissed?(TABS_POSITION_HIGHLIGHT) && !Rails.env.test?
+    current_user && !user_dismissed?(TABS_POSITION_HIGHLIGHT) && !Rails.env.test?
   end
 
   private

app/views/admin/broadcast_messages/_form.html.haml

-.broadcast-message-preview{ style: broadcast_message_style(@broadcast_message) }
-  = sprite_icon('bullhorn', size: 16, css_class:'vertical-align-text-top mr-2')
+.broadcast-banner-message.js-broadcast-banner-message-preview.mt-2{ style: broadcast_message_style(@broadcast_message), class: ('hidden' unless @broadcast_message.banner? ) }
+  = sprite_icon('bullhorn', size: 16, css_class:'vertical-align-text-top')
   .js-broadcast-message-preview
     - if @broadcast_message.message.present?
       = render_broadcast_message(@broadcast_message)
     - else
       Your message here
+- if Feature.enabled?(:broadcast_notification_type)
+  .d-flex.justify-content-center
+    .broadcast-notification-message.preview.js-broadcast-notification-message-preview.mt-2{ class: ('hidden' unless @broadcast_message.notification? ) }
+      = sprite_icon('bullhorn', size: 16, css_class:'vertical-align-text-top')
+      .js-broadcast-message-preview
+        - if @broadcast_message.message.present?
+          = render_broadcast_message(@broadcast_message)
+        - else
+          Your message here
 
 = form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form js-quick-submit js-requires-input'} do |f|
   = form_errors(@broadcast_message)
 
-  .form-group.row
+  .form-group.row.mt-4
     .col-sm-2.col-form-label
       = f.label :message
     .col-sm-10
-      = f.text_area :message, class: "form-control js-autosize",
+      = f.text_area :message, class: "form-control js-autosize js-broadcast-message-message",
         required: true,
         dir: 'auto',
         data: { preview_path: preview_admin_broadcast_messages_path }
-  .form-group.row
+  - if Feature.enabled?(:broadcast_notification_type)
+    .form-group.row
+      .col-sm-2.col-form-label
+        = f.label :broadcast_type, _('Type')
+      .col-sm-10
+        = f.select :broadcast_type, broadcast_type_options, {}, class: 'form-control js-broadcast-message-type'
+  .form-group.row.js-broadcast-message-background-color-form-group{ class: ('hidden' unless @broadcast_message.banner? ) }
     .col-sm-2.col-form-label
       = f.label :color, _("Background color")
     .col-sm-10
@@ -25,7 +40,7 @@
         .input-group-prepend
           .input-group-text.label-color-preview{ :style => 'background-color: ' + @broadcast_message.color + '; color: ' + @broadcast_message.font }
             = ' '.html_safe
-        = f.text_field :color, class: "form-control"
+        = f.text_field :color, class: "form-control js-broadcast-message-color"
       .form-text.text-muted
         = _('Choose any color.')
         %br

app/views/admin/broadcast_messages/index.html.haml

@@ -20,6 +20,7 @@
         %th Starts
         %th Ends
         %th Target Path
+        %th Type
         %th  
     %tbody
       - @broadcast_messages.each do |message|
@@ -27,13 +28,15 @@
           %td
             = broadcast_message_status(message)
           %td
-            = broadcast_message(message)
+            = broadcast_message(message, preview: true)
           %td
             = message.starts_at
           %td
             = message.ends_at
           %td
             = message.target_path
+          %td
+            = message.broadcast_type.capitalize
           %td
             = link_to sprite_icon('pencil-square'), edit_admin_broadcast_message_path(message), title: 'Edit', class: 'btn'
             = link_to sprite_icon('remove'), admin_broadcast_message_path(message), method: :delete, remote: true, title: 'Remove', class: 'js-remove-tr btn btn-danger'

app/views/layouts/_broadcast.html.haml

-- current_broadcast_messages&.each do |message|
+- current_broadcast_banner_messages.each do |message|
   = broadcast_message(message)
+- if Feature.enabled?(:broadcast_notification_type)
+  = broadcast_message(current_broadcast_notification_message)

app/views/projects/pages/_ssl_limitations_warning.html.haml

+.bs-callout.bs-callout-warning
+  %i.fa.fa-warning
+  %strong= _("Warning:")
+  - pages_host = Gitlab.config.pages.host
+  = s_("GitLabPages|When using Pages under the general domain of a GitLab instance (%{pages_host}), you cannot use HTTPS with sub-subdomains. This means that if your username/groupname contains a dot it will not work. This is a limitation of the HTTP Over TLS protocol. HTTP pages will continue to work provided you don't redirect HTTP to HTTPS.").html_safe % { pages_host: pages_host }
+
+  %strong= external_link(s_("GitLabPages|Learn more."), "https://docs.gitlab.com/ee/user/project/pages/introduction.html#limitations")

app/views/projects/pages/show.html.haml

@@ -15,6 +15,7 @@
 
   %hr.clearfix
 
+  = render 'ssl_limitations_warning' if @project.pages_subdomain.include?(".")
   = render 'access'
   = render 'use'
   - if Gitlab.config.pages.external_http || Gitlab.config.pages.external_https

changelogs/unreleased/30395-pages-ssl-limitations-warning.yml

+---
+title: Display SSL limitations warning for project's pages under namespace that contains
+  dot
+merge_request: 21874
+author:
+type: other

changelogs/unreleased/ph-195831-hideTabPopoverForAnonUsers.yml

+---
+title: Hide merge request tab popover for anonymous users
+merge_request: 22613
+author:
+type: fixed

changelogs/unreleased/prevent-job-logs-line-number-from-being-selected.yml

+---
+title: Prevent job log line numbers from being selected
+merge_request: 22691
+author:
+type: fixed

changelogs/unreleased/upgrade-kubeclient-4-4-0-to-4-6-0.yml

+---
+title: Bump kubeclient version from 4.4.0 to 4.6.0
+merge_request: 22347
+author:
+type: added

locale/gitlab.pot

@@ -8762,6 +8762,9 @@ msgstr ""
 msgid "GitLabPages|Learn how to upload your static site and have it served by GitLab by following the %{link_start}documentation on GitLab Pages%{link_end}."
 msgstr ""
 
+msgid "GitLabPages|Learn more."
+msgstr ""
+
 msgid "GitLabPages|Maximum size of pages (MB)"
 msgstr ""
 
@@ -8798,6 +8801,9 @@ msgstr ""
 msgid "GitLabPages|Verified"
 msgstr ""
 
+msgid "GitLabPages|When using Pages under the general domain of a GitLab instance (%{pages_host}), you cannot use HTTPS with sub-subdomains. This means that if your username/groupname contains a dot it will not work. This is a limitation of the HTTP Over TLS protocol. HTTP pages will continue to work provided you don't redirect HTTP to HTTPS."
+msgstr ""
+
 msgid "GitLabPages|With GitLab Pages you can host your static websites on GitLab.  Combined with the power of GitLab CI and the help of GitLab Runner you can deploy static pages for your individual projects, your user or your group."
 msgstr ""
 
@@ -20459,6 +20465,9 @@ msgstr ""
 msgid "Want to see the data? Please ask an administrator for access."
 msgstr ""
 
+msgid "Warning:"
+msgstr ""
+
 msgid "We could not determine the path to remove the epic"
 msgstr ""
 

qa/qa/page/base.rb

@@ -89,6 +89,10 @@ module QA
       end
 
       def all_elements(name, **kwargs)
+        if kwargs.keys.none? { |key| [:minimum, :maximum, :count, :between].include?(key) }
+          raise ArgumentError, "Please use :minimum, :maximum, :count, or :between so that all is more reliable"
+        end
+
         wait_for_requests
 
         all(element_selector_css(name), **kwargs)

qa/qa/page/component/note.rb

@@ -45,17 +45,17 @@ module QA
           click_element :comment_button
         end
 
-        def toggle_comments
-          all_elements(:toggle_comments_button).last.click
+        def toggle_comments(position)
+          all_elements(:toggle_comments_button, minimum: position)[position - 1].click
         end
 
-        def type_reply_to_discussion(reply_text)
-          all_elements(:discussion_reply_tab).last.click
+        def type_reply_to_discussion(position, reply_text)
+          all_elements(:discussion_reply_tab, minimum: position)[position - 1].click
           fill_element :reply_input, reply_text
         end
 
-        def reply_to_discussion(reply_text)
-          type_reply_to_discussion(reply_text)
+        def reply_to_discussion(position, reply_text)
+          type_reply_to_discussion(position, reply_text)
           click_element :reply_comment_button
         end
 

qa/qa/page/layout/performance_bar.rb

@@ -23,7 +23,7 @@ module QA
 
         def has_detailed_metrics?
           retry_until(sleep_interval: 1) do
-            all_elements(:detailed_metric_content).all? do |metric|
+            all_elements(:detailed_metric_content, count: count).all? do |metric|
               metric.has_text?(%r{\d+})
             end
           end

qa/qa/page/merge_request/show.rb

@@ -76,7 +76,7 @@ module QA
           wait(interval: 5) do
             has_text?("No newline at end of file")
           end
-          all_elements(:new_diff_line).first.hover
+          all_elements(:new_diff_line, minimum: 1).first.hover
           click_element(:diff_comment)
           fill_element(:reply_input, text)
         end

qa/qa/page/project/issue/index.rb

@@ -21,10 +21,6 @@ module QA
             element :closed_issues_link
           end
 
-          def assignee_link_count
-            all_elements(:assignee_link).count
-          end
-
           def avatar_counter
             find_element(:avatar_counter)
           end
@@ -37,6 +33,10 @@ module QA
             click_element :closed_issues_link
           end
 
+          def has_assignee_link_count?(count)
+            all_elements(:assignee_link, count: count)
+          end
+
           def has_issue?(issue)
             has_element? :issue, issue_title: issue.to_s
           end

qa/qa/page/project/issue/show.rb

@@ -56,12 +56,6 @@ module QA
             element :new_note_form, 'attr: :note' # rubocop:disable QA/ElementWithPattern
           end
 
-          def avatar_image_count
-            wait_assignees_block_finish_loading do
-              all_elements(:avatar_image).count
-            end
-          end
-
           def click_milestone_link
             click_element(:milestone_link)
           end
@@ -88,6 +82,12 @@ module QA
             click_element :comment_button
           end
 
+          def has_avatar_image_count?(count)
+            wait_assignees_block_finish_loading do
+              all_elements(:avatar_image, count: count)
+            end
+          end
+
           def has_comment?(comment_text)
             wait(reload: false) do
               has_element?(:noteable_note_item, text: comment_text)

qa/qa/page/project/settings/ci_variables.rb

@@ -20,13 +20,13 @@ module QA
           end
 
           def fill_variable(key, value, masked)
-            keys = all_elements(:ci_variable_input_key)
+            keys = all_elements(:ci_variable_input_key, minimum: 1)
             index = keys.size - 1
 
             # After we fill the key, JS would generate another field so
             # we need to use the same index to find the corresponding one.
             keys[index].set(key)
-            node = all_elements(:ci_variable_input_value)[index]
+            node = all_elements(:ci_variable_input_value, count: keys.size + 1)[index]
 
             # Simply run `node.set(value)` is too slow for long text here,
             # so we need to run JavaScript directly to set the value.
@@ -34,7 +34,7 @@ module QA
             # https://github.com/teamcapybara/capybara/blob/679548cea10773d45e32808f4d964377cfe5e892/lib/capybara/selenium/node.rb#L217
             execute_script("arguments[0].value = #{value.to_json}", node)
 
-            masked_node = all_elements(:variable_masked)[index]
+            masked_node = all_elements(:variable_masked, count: keys.size + 1)[index]
             toggle_masked(masked_node, masked)
           end
 

qa/qa/page/project/settings/mirroring_repositories.rb

@@ -118,7 +118,7 @@ module QA
 
           def find_repository_row_index(target_url)
             wait(max: 5, reload: false) do
-              all_elements(:mirror_repository_url_cell).index do |url|
+              all_elements(:mirror_repository_url_cell, minimum: 1).index do |url|
                 # The url might be a sanitized url but the target_url won't be so
                 # we compare just the paths instead of the full url
                 URI.parse(url.text).path == target_url.path

qa/qa/specs/features/browser_ui/2_plan/issue/collapse_comments_in_discussions_spec.rb

@@ -17,7 +17,7 @@ module QA
         Page::Project::Issue::Show.perform do |show|
           show.select_all_activities_filter
           show.start_discussion('My first discussion')
-          show.reply_to_discussion(my_first_reply)
+          show.reply_to_discussion(1, my_first_reply)
         end
       end
 

qa/qa/specs/features/browser_ui/non_devops/performance_bar_spec.rb

@@ -25,7 +25,7 @@ module QA
 
         Page::Layout::PerformanceBar.perform do |bar_component|
           expect(bar_component).to have_performance_bar
-          expect(bar_component).to have_detailed_metrics
+          expect(bar_component).to have_detailed_metrics(4)
           expect(bar_component).to have_request_for('realtime_changes') # Always requested on issue pages
         end
       end

qa/spec/page/base_spec.rb

@@ -91,4 +91,20 @@ describe QA::Page::Base do
       end
     end
   end
+
+  describe '#all_elements' do
+    before do
+      allow(subject).to receive(:all)
+    end
+
+    it 'raises an error if count or minimum are not specified' do
+      expect { subject.all_elements(:foo) }.to raise_error ArgumentError
+    end
+
+    it 'does not raise an error if :minimum, :maximum, :count, or :between is specified' do
+      [:minimum, :maximum, :count, :between].each do |param|
+        expect { subject.all_elements(:foo, param => 1) }.not_to raise_error
+      end
+    end
+  end
 end

qa/spec/page/logging_spec.rb

@@ -145,18 +145,18 @@ describe QA::Support::Page::Logging do
     it 'logs the number of elements found' do
       allow(page).to receive(:all).and_return([1, 2])
 
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 2) }
         .to output(/finding all :element/).to_stdout_from_any_process
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 2) }
         .to output(/found 2 :element/).to_stdout_from_any_process
     end
 
     it 'logs 0 if no elements are found' do
       allow(page).to receive(:all).and_return([])
 
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 1) }
         .to output(/finding all :element/).to_stdout_from_any_process
-      expect { subject.all_elements(:element) }
+      expect { subject.all_elements(:element, count: 1) }
         .not_to output(/found 0 :elements/).to_stdout_from_any_process
     end
   end

spec/controllers/projects/git_http_controller_spec.rb

@@ -90,6 +90,16 @@ describe Projects::GitHttpController do
       post :git_upload_pack, params: params
     end
 
+    context 'on a read-only instance' do
+      before do
+        allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+      end
+
+      it 'does not update project statistics' do
+        expect(ProjectDailyStatisticsWorker).not_to receive(:perform_async)
+      end
+    end
+
     it 'updates project statistics' do
       expect(ProjectDailyStatisticsWorker).to receive(:perform_async)
     end

spec/features/admin/admin_broadcast_messages_spec.rb

@@ -13,7 +13,7 @@ describe 'Admin Broadcast Messages' do
     expect(page).to have_content 'Migration to new server'
   end
 
-  it 'Create a customized broadcast message' do
+  it 'creates a customized broadcast banner message' do
     fill_in 'broadcast_message_message', with: 'Application update from **4:00 CST to 5:00 CST**'
     fill_in 'broadcast_message_color', with: '#f2dede'
     fill_in 'broadcast_message_target_path', with: '*/user_onboarded'
@@ -28,6 +28,20 @@ describe 'Admin Broadcast Messages' do
     expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"])
   end
 
+  it 'creates a customized broadcast notification message' do
+    fill_in 'broadcast_message_message', with: 'Application update from **4:00 CST to 5:00 CST**'
+    fill_in 'broadcast_message_target_path', with: '*/user_onboarded'
+    select 'Notification', from: 'broadcast_message_broadcast_type'
+    select Date.today.next_year.year, from: 'broadcast_message_ends_at_1i'
+    click_button 'Add broadcast message'
+
+    expect(current_path).to eq admin_broadcast_messages_path
+    expect(page).to have_content 'Application update from 4:00 CST to 5:00 CST'
+    expect(page).to have_content '*/user_onboarded'
+    expect(page).to have_content 'Notification'
+    expect(page).to have_selector 'strong', text: '4:00 CST to 5:00 CST'
+  end
+
   it 'Edit an existing broadcast message' do
     click_link 'Edit'
     fill_in 'broadcast_message_message', with: 'Application update RIGHT NOW'
@@ -44,10 +58,20 @@ describe 'Admin Broadcast Messages' do
     expect(page).not_to have_content 'Migration to new server'
   end
 
-  it 'Live preview a customized broadcast message', :js do
+  it 'updates a preview of a customized broadcast banner message', :js do
+    fill_in 'broadcast_message_message', with: "Live **Markdown** previews. :tada:"
+
+    page.within('.js-broadcast-banner-message-preview') do
+      expect(page).to have_selector('strong', text: 'Markdown')
+      expect(page).to have_emoji('tada')
+    end
+  end
+
+  it 'updates a preview of a customized broadcast notification message', :js do
     fill_in 'broadcast_message_message', with: "Live **Markdown** previews. :tada:"
+    select 'Notification', from: 'broadcast_message_broadcast_type'
 
-    page.within('.broadcast-message-preview') do
+    page.within('.js-broadcast-notification-message-preview') do
       expect(page).to have_selector('strong', text: 'Markdown')
       expect(page).to have_emoji('tada')
     end

spec/helpers/broadcast_messages_helper_spec.rb

@@ -4,24 +4,22 @@ require 'spec_helper'
 
 describe BroadcastMessagesHelper do
   describe 'broadcast_message' do
+    let(:current_broadcast_message) { BroadcastMessage.new(message: 'Current Message') }
+
     it 'returns nil when no current message' do
       expect(helper.broadcast_message(nil)).to be_nil
     end
 
     it 'includes the current message' do
-      current = BroadcastMessage.new(message: 'Current Message')
-
       allow(helper).to receive(:broadcast_message_style).and_return(nil)
 
-      expect(helper.broadcast_message(current)).to include 'Current Message'
+      expect(helper.broadcast_message(current_broadcast_message)).to include 'Current Message'
     end
 
     it 'includes custom style' do
-      current = BroadcastMessage.new(message: 'Current Message')
-
       allow(helper).to receive(:broadcast_message_style).and_return('foo')
 
-      expect(helper.broadcast_message(current)).to include 'style="foo"'
+      expect(helper.broadcast_message(current_broadcast_message)).to include 'style="foo"'
     end
   end
 
@@ -32,12 +30,18 @@ describe BroadcastMessagesHelper do
       expect(helper.broadcast_message_style(broadcast_message)).to eq ''
     end
 
-    it 'allows custom style' do
-      broadcast_message = double(color: '#f2dede', font: '#b94a48')
+    it 'allows custom style for banner messages' do
+      broadcast_message = BroadcastMessage.new(color: '#f2dede', font: '#b94a48', broadcast_type: "banner")
 
       expect(helper.broadcast_message_style(broadcast_message))
         .to match('background-color: #f2dede; color: #b94a48')
     end
+
+    it 'does not add style for notification messages' do
+      broadcast_message = BroadcastMessage.new(color: '#f2dede', broadcast_type: "notification")
+
+      expect(helper.broadcast_message_style(broadcast_message)).to eq ''
+    end
   end
 
   describe 'broadcast_message_status' do