fix: weekly rotation browser push, Slack title escaping, comment cleanup

apps/notifications/src/__tests__/mappings/weeklyRotation.test.ts

@@ -81,7 +81,7 @@ describe('Weekly Rotation Notification', () => {
     )
   })
 
-  test('Does not push to a user with no devices', async () => {
+  test('Sends a browser push but no mobile push to a user with no devices', async () => {
     await createUsers(processor.discoveryDB, [{ user_id: 2 }])
 
     await insertNotifications(processor.discoveryDB, [
@@ -101,5 +101,12 @@ describe('Weekly Rotation Notification', () => {
     await processor.appNotificationsProcessor.process(pending.appNotifications)
 
     expect(sendPushNotificationSpy).not.toHaveBeenCalled()
+    expect(sendBrowserNotificationSpy).toHaveBeenCalledWith(
+      true,
+      expect.any(Object),
+      2,
+      weeklyRotationMessages.title,
+      weeklyRotationMessages.body
+    )
   })
 })

apps/notifications/src/processNotifications/mappers/weeklyRotation.ts

@@ -16,20 +16,16 @@ type WeeklyRotationNotificationRow = Omit<NotificationRow, 'data'> & {
 
 export const weeklyRotationMessages = {
   title: '🎧 Your Weekly Rotation Is Ready',
-  body: 'A fresh mix of tracks picked just for you. Give it a spin before it rotates next Wednesday.'
+  body: 'A fresh mix of tracks picked just for you. Updates every Wednesday.'
 }
 
 /**
- * Announces the Wednesday Weekly Rotation rollover.
+ * Push for the Wednesday Weekly Rotation rollover. Rows are written once per
+ * listener per period by the api's WeeklyRotationNotificationsJob
+ * (group_id `weekly_rotation:<YYYY-WW>:<user>`).
  *
- * Rows are written once per listener per period by the api repo's
- * WeeklyRotationNotificationsJob (group_id `weekly_rotation:<YYYY-WW>:<user>`).
- * There is no entity to link: the push and the in-app tile both open the
- * listener's own mix, which the client fetches on demand.
- *
- * Deliberately no rich image. The og collage for a mix is rendered from the
- * mix itself, so attaching it would make every device that receives the push
- * compute its owner's rotation at once.
+ * No rich image: the og collage is rendered from the mix, so attaching it
+ * would compute a mix per recipient.
  */
 export class WeeklyRotation extends BaseNotification<WeeklyRotationNotificationRow> {
   receiverUserId: number
@@ -56,6 +52,14 @@ export class WeeklyRotation extends BaseNotification<WeeklyRotationNotificationR
 
     const { title, body } = weeklyRotationMessages
 
+    await sendBrowserNotification(
+      isBrowserPushEnabled,
+      userNotificationSettings,
+      this.receiverUserId,
+      title,
+      body
+    )
+
     if (
       !userNotificationSettings.shouldSendPushNotification({
         initiatorUserId: this.receiverUserId,
@@ -65,18 +69,9 @@ export class WeeklyRotation extends BaseNotification<WeeklyRotationNotificationR
       return
     }
 
-    await sendBrowserNotification(
-      isBrowserPushEnabled,
-      userNotificationSettings,
-      this.receiverUserId,
-      title,
-      body
-    )
-
     const devices: Device[] = userNotificationSettings.getDevices(
       this.receiverUserId
     )
-    if (devices.length === 0) return
 
     const pushes = await Promise.all(
       devices.map((device) =>

apps/notifications/src/tasks/dmNotifications.ts

@@ -247,12 +247,9 @@ export async function getNewBlasts(
   }
 }
 
-// Chats the receiver has filed into their General inbox (chat.set_category)
-// are silent: the message still lands in the inbox and lights the General tab
-// dot in the clients, but no push or browser notification is sent. A chat with
-// no preference row (uncategorized) or filed as Priority notifies as normal.
-// The preference is per receiver, so the same chat can be silent for one
-// member and loud for the other.
+// Chats the receiver filed as General (chat.set_category) get no push or
+// browser notification. Uncategorized and Priority chats notify as usual.
+// The category is per receiver.
 export const generalInboxKey = (receiverUserId: number, chatId: string) =>
   `${receiverUserId}:${chatId}`
 

apps/trending-challenge-rewards/src/__tests__/trending.test.ts

@@ -135,7 +135,7 @@ describe('queryHandles', () => {
       twitter: '@third_prefixed',
       instagram: '@insta_prefixed'
     })
-    // users missing from discovery keep the synthetic fallback, no instagram
+    // users missing from discovery fall back to @/user-<id> with no instagram
     expect(handles.get(4)).toEqual({ twitter: '@/user-4' })
   })
 })
@@ -202,7 +202,7 @@ describe('queryTrackLinks', () => {
       title: 'First Track',
       url: 'https://audius.co/artistOne/first-track'
     })
-    // rows without a current route are skipped rather than linked to nowhere
+    // rows without a current route are skipped
     expect(links.get(11)).toBeUndefined()
   })
 
@@ -256,15 +256,29 @@ describe('composeTrackLinks', () => {
 
     expect(out).toContain('*Top 10 Trending Tracks 🔥*')
     expect(out).toContain(
-      '1. @first — <https://audius.co/a/track-one|Track One>'
+      '1. @first - <https://audius.co/a/track-one|Track One>'
     )
     expect(out.indexOf('Track One')).toBeLessThan(out.indexOf('Track Two'))
   })
 
+  it('escapes slack control characters in track titles', () => {
+    const out = composeTrackLinks('Top 10 Trending Tracks 🔥', [
+      {
+        handle: '@first',
+        rank: 1,
+        title: 'Up > Down & <Left>',
+        url: 'https://audius.co/a/up-down'
+      }
+    ])
+    expect(out).toContain(
+      '<https://audius.co/a/up-down|Up &gt; Down &amp; &lt;Left&gt;>'
+    )
+  })
+
   it('falls back to a placeholder when the track link is missing', () => {
     const out = composeTrackLinks('Top 10 Trending Underground 🎵', [
       { handle: '@first', rank: 1 }
     ])
-    expect(out).toContain('1. @first — _track link unavailable_')
+    expect(out).toContain('1. @first - _track link unavailable_')
   })
 })

apps/trending-challenge-rewards/src/trending.ts

@@ -277,18 +277,23 @@ export const composeTweet = (
   return '```\n' + `${title} (${week})` + newLine + rows + '```'
 }
 
-// Companion to the tweet block: the same winners with the track that actually
-// won and a clickable link, so artists with several trending tracks can be
-// told apart. Kept outside the code fence so Slack renders the links.
+// Slack mrkdwn requires &, < and > to be escaped in message text.
+const escapeSlack = (text: string): string =>
+  text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
+
+// Winners with their track title and link. Outside the code block so Slack
+// renders the links.
 export const composeTrackLinks = (
   title: string,
   entries: TrendingEntry[]
 ): string => {
   const lines = byRank(entries).map((entry) => {
     if (entry.url === undefined || entry.title === undefined) {
-      return `${entry.rank}. ${entry.handle} — _track link unavailable_`
+      return `${entry.rank}. ${entry.handle} - _track link unavailable_`
     }
-    return `${entry.rank}. ${entry.handle} — <${entry.url}|${entry.title}>`
+    return `${entry.rank}. ${entry.handle} - <${entry.url}|${escapeSlack(
+      entry.title
+    )}>`
   })
   return [`*${title}*`, ...lines].join('\n')
 }