Add user dismiss option to admin broadcast messages

This commit introduces a new configuration option for broadcast
messages to make them user dismissable. This option can be
enabled/disabled per broadcast message by any instance
administrator.

app/assets/javascripts/broadcast_notification.js

@@ -6,7 +6,7 @@ const handleOnDismiss = ({ currentTarget }) => {
     dataset: { id },
   } = currentTarget;
 
-  Cookies.set(`hide_broadcast_notification_message_${id}`, true);
+  Cookies.set(`hide_broadcast_message_${id}`, true);
 
   const notification = document.querySelector(`.js-broadcast-notification-${id}`);
   notification.parentNode.removeChild(notification);

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

@@ -25,9 +25,11 @@ export default () => {
 
   $broadcastMessageType.on('change', () => {
     const $broadcastMessageColorFormGroup = $('.js-broadcast-message-background-color-form-group');
+    const $broadcastMessageDismissableFormGroup = $('.js-broadcast-message-dismissable-form-group');
     const $broadcastNotificationMessagePreview = $('.js-broadcast-notification-message-preview');
 
     $broadcastMessageColorFormGroup.toggleClass('hidden');
+    $broadcastMessageDismissableFormGroup.toggleClass('hidden');
     $broadcastBannerMessagePreview.toggleClass('hidden');
     $broadcastNotificationMessagePreview.toggleClass('hidden');
   });

app/assets/stylesheets/framework/broadcast_messages.scss

@@ -17,6 +17,10 @@
   @extend .broadcast-message;
   @extend .alert-warning;
   text-align: center;
+
+  .broadcast-message-dismiss {
+    color: inherit;
+  }
 }
 
 .broadcast-notification-message {
@@ -36,6 +40,10 @@
   &.preview {
     position: static;
   }
+
+  .broadcast-message-dismiss {
+    color: $gray-800;
+  }
 }
 
 .toggle-colors {

app/controllers/admin/broadcast_messages_controller.rb

@@ -62,6 +62,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
       starts_at
       target_path
       broadcast_type
+      dismissable
     ))
   end
 end

app/helpers/broadcast_messages_helper.rb

@@ -2,12 +2,14 @@
 
 module BroadcastMessagesHelper
   def current_broadcast_banner_messages
-    BroadcastMessage.current_banner_messages(request.path)
+    BroadcastMessage.current_banner_messages(request.path).select do |message|
+      cookies["hide_broadcast_message_#{message.id}"].blank?
+    end
   end
 
   def current_broadcast_notification_message
     not_hidden_messages = BroadcastMessage.current_notification_messages(request.path).select do |message|
-      cookies["hide_broadcast_notification_message_#{message.id}"].blank?
+      cookies["hide_broadcast_message_#{message.id}"].blank?
     end
     not_hidden_messages.last
   end

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

@@ -46,6 +46,13 @@
 
       = render_suggested_colors
 
+  .form-group.row.js-broadcast-message-dismissable-form-group{ class: ('hidden' unless @broadcast_message.banner? ) }
+    .col-sm-2.col-form-label.pt-0
+      = f.label :starts_at, _("Dismissable")
+    .col-sm-10
+      = f.check_box :dismissable
+      = f.label :dismissable do
+        = _('Allow users to dismiss the broadcast message')
   .form-group.row.js-toggle-colors-container.toggle-colors.hide
     .col-sm-2.col-form-label
       = f.label :font, "Font Color"

app/views/shared/_broadcast_message.html.haml

@@ -3,6 +3,6 @@
   %div
     = sprite_icon('bullhorn', size: 16, css_class: 'vertical-align-text-top')
     = render_broadcast_message(message)
-  - if message.notification? && opts[:preview].blank?
-    %button.js-dismiss-current-broadcast-notification.btn.btn-link.text-dark.pl-2.pr-2{ 'aria-label' => _('Close'), :type => 'button', data: { id: message.id } }
+  - if (message.notification? || message.dismissable?) && opts[:preview].blank?
+    %button.broadcast-message-dismiss.js-dismiss-current-broadcast-notification.btn.btn-link.pl-2.pr-2{ 'aria-label' => _('Close'), :type => 'button', data: { id: message.id } }
       %i.fa.fa-times

db/migrate/20191123081456_add_dismissable_to_broadcast_messages.rb

+# frozen_string_literal: true
+
+class AddDismissableToBroadcastMessages < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :broadcast_messages, :dismissable, :boolean
+  end
+end

db/schema.rb

@@ -573,6 +573,7 @@ ActiveRecord::Schema.define(version: 2020_03_03_074328) do
     t.integer "cached_markdown_version"
     t.string "target_path", limit: 255
     t.integer "broadcast_type", limit: 2, default: 1, null: false
+    t.boolean "dismissable"
     t.index ["ends_at", "broadcast_type", "id"], name: "index_broadcast_message_on_ends_at_and_broadcast_type_and_id"
   end
 

doc/api/broadcast_messages.md

@@ -36,7 +36,8 @@ Example response:
         "id":1,
         "active": false,
         "target_path": "*/welcome",
-        "broadcast_type": "banner"
+        "broadcast_type": "banner",
+        "dismissable": false
     }
 ]
 ```
@@ -73,7 +74,8 @@ Example response:
     "id":1,
     "active":false,
     "target_path": "*/welcome",
-    "broadcast_type": "banner"
+    "broadcast_type": "banner",
+    "dismissable": false
 }
 ```
 
@@ -87,15 +89,16 @@ POST /broadcast_messages
 
 Parameters:
 
-| Attribute   | Type     | Required | Description                                           |
-|:------------|:---------|:---------|:------------------------------------------------------|
-| `message`   | string   | yes      | Message to display.                                   |
-| `starts_at` | datetime | no       | Starting time (defaults to current time).             |
-| `ends_at`   | datetime | no       | Ending time (defaults to one hour from current time). |
-| `color`     | string   | no       | Background color hex code.                            |
-| `font`      | string   | no       | Foreground color hex code.                            |
-| `target_path`| string  | no       | Target path of the broadcast message.                 |
-| `broadcast_type`| string | no     | Appearance type (defaults to banner)                  |
+| Attribute       | Type     | Required | Description                                           |
+|:----------------|:---------|:---------|:------------------------------------------------------|
+| `message`       | string   | yes      | Message to display.                                   |
+| `starts_at`     | datetime | no       | Starting time (defaults to current time).             |
+| `ends_at`       | datetime | no       | Ending time (defaults to one hour from current time). |
+| `color`         | string   | no       | Background color hex code.                            |
+| `font`          | string   | no       | Foreground color hex code.                            |
+| `target_path`   | string   | no       | Target path of the broadcast message.                 |
+| `broadcast_type`| string   | no       | Appearance type (defaults to banner)                  |
+| `dismissable`   | boolean  | no       | Can the user dismiss the message?                     |
 
 Example request:
 
@@ -116,6 +119,7 @@ Example response:
     "active": true,
     "target_path": "*/welcome",
     "broadcast_type": "notification",
+    "dismissable": false
 }
 ```
 
@@ -129,16 +133,17 @@ PUT /broadcast_messages/:id
 
 Parameters:
 
-| Attribute   | Type     | Required | Description                        |
-|:------------|:---------|:---------|:-----------------------------------|
-| `id`        | integer  | yes      | ID of broadcast message to update. |
-| `message`   | string   | no       | Message to display.                |
-| `starts_at` | datetime | no       | Starting time.                     |
-| `ends_at`   | datetime | no       | Ending time.                       |
-| `color`     | string   | no       | Background color hex code.         |
-| `font`      | string   | no       | Foreground color hex code.         |
-| `target_path`| string  | no       | Target path of the broadcast message. |
-| `broadcast_type`| string | no     | Appearance type (defaults to banner) |
+| Attribute       | Type     | Required | Description                           |
+|:----------------|:---------|:---------|:--------------------------------------|
+| `id`            | integer  | yes      | ID of broadcast message to update.    |
+| `message`       | string   | no       | Message to display.                   |
+| `starts_at`     | datetime | no       | Starting time.                        |
+| `ends_at`       | datetime | no       | Ending time.                          |
+| `color`         | string   | no       | Background color hex code.            |
+| `font`          | string   | no       | Foreground color hex code.            |
+| `target_path`   | string   | no       | Target path of the broadcast message. |
+| `broadcast_type`| string   | no       | Appearance type (defaults to banner)  |
+| `dismissable`   | boolean  | no       | Can the user dismiss the message?     |
 
 Example request:
 
@@ -159,6 +164,7 @@ Example response:
     "active": true,
     "target_path": "*/welcome",
     "broadcast_type": "notification",
+    "dismissable": false
 }
 ```
 

doc/user/admin_area/broadcast_messages.md

@@ -28,7 +28,7 @@ To add a broadcast message:
 
 NOTE: **Note:**
 Once a broadcast message has expired, it is no longer displayed in the UI but is still listed in the
-list of broadcast messages.
+list of broadcast messages. User can also dismiss a broadcast message if the option **Dismissable** is set.
 
 ## Editing a broadcast message
 

lib/api/broadcast_messages.rb

@@ -36,6 +36,7 @@ module API
         optional :font, type: String, desc: 'Foreground color'
         optional :target_path, type: String, desc: 'Target path'
         optional :broadcast_type, type: String, values: BroadcastMessage.broadcast_types.keys, desc: 'Broadcast type. Defaults to banner', default: -> { 'banner' }
+        optional :dismissable, type: Boolean, desc: 'Is dismissable'
       end
       post do
         authenticated_as_admin!
@@ -75,6 +76,7 @@ module API
         optional :font, type: String, desc: 'Foreground color'
         optional :target_path, type: String, desc: 'Target path'
         optional :broadcast_type, type: String, values: BroadcastMessage.broadcast_types.keys, desc: 'Broadcast Type'
+        optional :dismissable, type: Boolean, desc: 'Is dismissable'
       end
       put ':id' do
         authenticated_as_admin!

lib/api/entities/broadcast_message.rb

@@ -3,7 +3,7 @@
 module API
   module Entities
     class BroadcastMessage < Grape::Entity
-      expose :id, :message, :starts_at, :ends_at, :color, :font, :target_path, :broadcast_type
+      expose :id, :message, :starts_at, :ends_at, :color, :font, :target_path, :broadcast_type, :dismissable
       expose :active?, as: :active
     end
   end

locale/gitlab.pot

@@ -1666,6 +1666,9 @@ msgstr ""
 msgid "Allow this secondary node to replicate content on Object Storage"
 msgstr ""
 
+msgid "Allow users to dismiss the broadcast message"
+msgstr ""
+
 msgid "Allow users to register any application to use GitLab as an OAuth provider"
 msgstr ""
 
@@ -6910,6 +6913,9 @@ msgstr ""
 msgid "Dismiss trial promotion"
 msgstr ""
 
+msgid "Dismissable"
+msgstr ""
+
 msgid "Dismissed"
 msgstr ""
 

spec/features/admin/admin_broadcast_messages_spec.rb

@@ -18,6 +18,7 @@ describe 'Admin Broadcast Messages' do
     fill_in 'broadcast_message_color', with: '#f2dede'
     fill_in 'broadcast_message_target_path', with: '*/user_onboarded'
     fill_in 'broadcast_message_font', with: '#b94a48'
+    check 'broadcast_message_dismissable'
     select Date.today.next_year.year, from: 'broadcast_message_ends_at_1i'
     click_button 'Add broadcast message'
 
@@ -26,6 +27,7 @@ describe 'Admin Broadcast Messages' do
     expect(page).to have_content '*/user_onboarded'
     expect(page).to have_selector 'strong', text: '4:00 CST to 5:00 CST'
     expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"])
+    expect(page).to have_selector 'a', text: 'Dismiss'
   end
 
   it 'creates a customized broadcast notification message' do

spec/features/broadcast_messages_spec.rb

@@ -3,28 +3,58 @@
 require 'spec_helper'
 
 describe 'Broadcast Messages' do
-  let!(:broadcast_message) { create(:broadcast_message, broadcast_type: 'notification', message: 'SampleMessage') }
+  shared_examples 'a Broadcast Messages' do
+    it 'shows broadcast message' do
+      visit root_path
+  
+      expect(page).to have_content 'SampleMessage'
+    end
+  end
+
+  shared_examples 'a dismissable Broadcast Messages' do
+    it 'hides broadcast message after dismiss', :js do
+      visit root_path
+  
+      find('.js-dismiss-current-broadcast-notification').click
+  
+      expect(page).not_to have_content 'SampleMessage'
+    end
+  
+    it 'broadcast message is still hidden after refresh', :js do
+      visit root_path
+  
+      find('.js-dismiss-current-broadcast-notification').click
+      visit root_path
+  
+      expect(page).not_to have_content 'SampleMessage'
+    end
+  end
+
+  describe 'banner type' do
+    let!(:broadcast_message) { create(:broadcast_message, message: 'SampleMessage') }
+
+    it_behaves_like 'a Broadcast Messages'
 
-  it 'shows broadcast message' do
-    visit root_path
+    it 'shows broadcast message' do
+      visit root_path
 
-    expect(page).to have_content 'SampleMessage'
+      expect(page).not_to have_selector('.js-dismiss-current-broadcast-notification')
+    end
   end
 
-  it 'hides broadcast message after dismiss', :js do
-    visit root_path
+  describe 'dismissable banner type' do
+    let!(:broadcast_message) { create(:broadcast_message, dismissable: true, message: 'SampleMessage') }
 
-    find('.js-dismiss-current-broadcast-notification').click
+    it_behaves_like 'a Broadcast Messages'
 
-    expect(page).not_to have_content 'SampleMessage'
+    it_behaves_like 'a dismissable Broadcast Messages'
   end
 
-  it 'broadcast message is still hidden after refresh', :js do
-    visit root_path
+  describe 'notification type' do
+    let!(:broadcast_message) { create(:broadcast_message, broadcast_type: 'notification', message: 'SampleMessage') }
 
-    find('.js-dismiss-current-broadcast-notification').click
-    visit root_path
+    it_behaves_like 'a Broadcast Messages'
 
-    expect(page).not_to have_content 'SampleMessage'
+    it_behaves_like 'a dismissable Broadcast Messages'
   end
 end

spec/helpers/broadcast_messages_helper_spec.rb

@@ -14,7 +14,7 @@ describe BroadcastMessagesHelper do
 
       context 'when last broadcast message is hidden' do
         before do
-          helper.request.cookies["hide_broadcast_notification_message_#{broadcast_message_2.id}"] = 'true'
+          helper.request.cookies["hide_broadcast_message_#{broadcast_message_2.id}"] = 'true'
         end
 
         it { is_expected.to eq broadcast_message_1 }

spec/requests/api/broadcast_messages_spec.rb

@@ -17,7 +17,7 @@ describe API::BroadcastMessages do
       expect(response).to include_pagination_headers
       expect(json_response).to be_kind_of(Array)
       expect(json_response.first.keys)
-        .to match_array(%w(id message starts_at ends_at color font active target_path broadcast_type))
+        .to match_array(%w(id message starts_at ends_at color font active target_path broadcast_type dismissable))
     end
   end
 
@@ -28,7 +28,7 @@ describe API::BroadcastMessages do
       expect(response).to have_gitlab_http_status(:ok)
       expect(json_response['id']).to eq message.id
       expect(json_response.keys)
-        .to match_array(%w(id message starts_at ends_at color font active target_path broadcast_type))
+        .to match_array(%w(id message starts_at ends_at color font active target_path broadcast_type dismissable))
     end
   end
 
@@ -111,6 +111,15 @@ describe API::BroadcastMessages do
 
         expect(response).to have_gitlab_http_status(:bad_request)
       end
+
+      it 'accepts an active dismissable value ' do
+        attrs = { message: 'new message', dismissable: true }
+
+        post api('/broadcast_messages', admin), params: attrs
+
+        expect(response).to have_gitlab_http_status(201)
+        expect(json_response['dismissable']).to eq true
+      end
     end
   end
 
@@ -187,6 +196,15 @@ describe API::BroadcastMessages do
 
         expect(response).to have_gitlab_http_status(:bad_request)
       end
+
+      it 'accepts a new dismissable value ' do
+        attrs = { message: 'new message', dismissable: true }
+
+        put api("/broadcast_messages/#{message.id}", admin), params: attrs
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['dismissable']).to eq true
+      end
     end
   end