Merge branch 'nicolasdular/add-target-path-to-broadcast-message' into 'master'

Add path based targeting to broadcast messages

See merge request gitlab-org/gitlab!20191

app/controllers/admin/broadcast_messages_controller.rb

@@ -60,6 +60,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
       font
       message
       starts_at
+      target_path
     ))
   end
 end

app/helpers/broadcast_messages_helper.rb

 # frozen_string_literal: true
 
 module BroadcastMessagesHelper
+  def current_broadcast_messages
+    BroadcastMessage.current(request.path)
+  end
+
   def broadcast_message(message)
     return unless message.present?
 

app/models/broadcast_message.rb

@@ -20,7 +20,7 @@ class BroadcastMessage < ApplicationRecord
 
   after_commit :flush_redis_cache
 
-  def self.current
+  def self.current(current_path = nil)
     messages = cache.fetch(CACHE_KEY, as: BroadcastMessage, expires_in: cache_expires_in) do
       current_and_future_messages
     end
@@ -33,7 +33,7 @@ class BroadcastMessage < ApplicationRecord
     # cache so we don't keep running this code all the time.
     cache.expire(CACHE_KEY) if now_or_future.empty?
 
-    now_or_future.select(&:now?)
+    now_or_future.select(&:now?).select { |message| message.matches_current_path(current_path) }
   end
 
   def self.current_and_future_messages
@@ -72,6 +72,12 @@ class BroadcastMessage < ApplicationRecord
     now? || future?
   end
 
+  def matches_current_path(current_path)
+    return true if current_path.blank? || target_path.blank?
+
+    current_path.match(Regexp.escape(target_path).gsub('\\*', '.*'))
+  end
+
   def flush_redis_cache
     self.class.cache.expire(CACHE_KEY)
   end

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

@@ -38,6 +38,13 @@
       = f.label :font, "Font Color"
     .col-sm-10
       = f.color_field :font, class: "form-control text-font-color"
+  .form-group.row
+    .col-sm-2.col-form-label
+      = f.label :target_path, _('Target Path')
+    .col-sm-10
+      = f.text_field :target_path, class: "form-control"
+      .form-text.text-muted
+        = _('Paths can contain wildcards, like */welcome')
   .form-group.row
     .col-sm-2.col-form-label
       = f.label :starts_at, _("Starts at (UTC)")

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

@@ -19,6 +19,7 @@
         %th Preview
         %th Starts
         %th Ends
+        %th Target Path
         %th  
     %tbody
       - @broadcast_messages.each do |message|
@@ -31,6 +32,8 @@
             = message.starts_at
           %td
             = message.ends_at
+          %td
+            = message.target_path
           %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

-- BroadcastMessage.current&.each do |message|
+- current_broadcast_messages&.each do |message|
   = broadcast_message(message)

changelogs/unreleased/nicolasdular-add-target-path-to-broadcast-message.yml

+---
+title: Add path based targeting to broadcast messages
+merge_request:
+author:
+type: added

db/migrate/20191125133353_add_target_path_to_broadcast_message.rb

+# frozen_string_literal: true
+
+class AddTargetPathToBroadcastMessage < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :broadcast_messages, :target_path, :string, limit: 255
+  end
+end

db/schema.rb

@@ -573,6 +573,7 @@ ActiveRecord::Schema.define(version: 2019_11_25_140458) do
     t.string "font"
     t.text "message_html", null: false
     t.integer "cached_markdown_version"
+    t.string "target_path", limit: 255
     t.index ["starts_at", "ends_at", "id"], name: "index_broadcast_messages_on_starts_at_and_ends_at_and_id"
   end
 

doc/user/admin_area/broadcast_messages.md

@@ -22,6 +22,7 @@ To add a broadcast message:
 1. Navigate to the **Admin Area > Messages** page.
 1. Add the text for the message to the **Message** field. Markdown and emoji are supported.
 1. If required, click the **Customize colors** link to edit the background color and font color of the message.
+1. If required, add a **Target Path** to only show the broadcast message on URLs matching that path. You can use the wildcard character `*` to match multiple URLs, for example `/users/*/issues`.
 1. Select a date for the message to start and end.
 1. Click the **Add broadcast message** button.
 

locale/gitlab.pot

@@ -12263,6 +12263,9 @@ msgstr ""
 msgid "Path:"
 msgstr ""
 
+msgid "Paths can contain wildcards, like */welcome"
+msgstr ""
+
 msgid "Pause"
 msgstr ""
 
@@ -17109,6 +17112,9 @@ msgstr ""
 msgid "Target Branch"
 msgstr ""
 
+msgid "Target Path"
+msgstr ""
+
 msgid "Target branch"
 msgstr ""
 

spec/features/admin/admin_broadcast_messages_spec.rb

@@ -16,12 +16,14 @@ describe 'Admin Broadcast Messages' do
   it 'Create a customized broadcast 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'
     fill_in 'broadcast_message_font', with: '#b94a48'
     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_selector 'strong', text: '4:00 CST to 5:00 CST'
     expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"])
   end

spec/models/broadcast_message_spec.rb

@@ -88,6 +88,42 @@ describe BroadcastMessage do
       expect(Rails.cache).not_to receive(:delete).with(described_class::CACHE_KEY)
       expect(described_class.current.length).to eq(0)
     end
+
+    it 'returns message if it matches the target path' do
+      message = create(:broadcast_message, target_path: "*/onboarding_completed")
+
+      expect(described_class.current('/users/onboarding_completed')).to include(message)
+    end
+
+    it 'returns message if part of the target path matches' do
+      create(:broadcast_message, target_path: "/users/*/issues")
+
+      expect(described_class.current('/users/name/issues').length).to eq(1)
+    end
+
+    it 'returns the message for empty target path' do
+      create(:broadcast_message, target_path: "")
+
+      expect(described_class.current('/users/name/issues').length).to eq(1)
+    end
+
+    it 'returns the message if target path is nil' do
+      create(:broadcast_message, target_path: nil)
+
+      expect(described_class.current('/users/name/issues').length).to eq(1)
+    end
+
+    it 'does not return message if target path does not match' do
+      create(:broadcast_message, target_path: "/onboarding_completed")
+
+      expect(described_class.current('/welcome').length).to eq(0)
+    end
+
+    it 'does not return message if target path does not match when using wildcard' do
+      create(:broadcast_message, target_path: "/users/*/issues")
+
+      expect(described_class.current('/group/groupname/issues').length).to eq(0)
+    end
   end
 
   describe '#attributes' do