From cd412056ad33732ba795d80f23de984c9268017f Mon Sep 17 00:00:00 2001
From: Wyatt Johnson <wyattjoh@gmail.com>
Date: Mon, 26 Mar 2018 15:01:55 -0600
Subject: [PATCH] Notification fixes

- When a comment reply is not visible, it will not trigger a
notification email.
- When a comment is approved/accepted, we'll check to see if the comment
was visible before, if not, we'll delegate to the new reply notification
handler to possibly send a notification email.
---
 .../index.js                                  | 54 ++++++++++++++++---
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/plugins/talk-plugin-notifications-category-reply/index.js b/plugins/talk-plugin-notifications-category-reply/index.js
index 0b6fe62a0..e214975ac 100644
--- a/plugins/talk-plugin-notifications-category-reply/index.js
+++ b/plugins/talk-plugin-notifications-category-reply/index.js
@@ -2,6 +2,12 @@ const { get } = require('lodash');
 const path = require('path');
 
 const handle = async (ctx, comment) => {
+  // Check to see if this reply is visible.
+  if (!comment.visible) {
+    ctx.log.info('comment was not visible, not sending notification');
+    return;
+  }
+
   // Check to see if this is a reply to an existing comment.
   const parentID = get(comment, 'parent_id', null);
   if (parentID === null) {
@@ -90,12 +96,31 @@ const hydrate = async (ctx, category, context) => {
   return [headline, replier, permalink];
 };
 
-const handler = {
-  handle,
-  category: 'reply',
-  event: 'commentAdded',
-  hydrate,
-  digestOrder: 30,
+// commentAcceptedHandleAdapter will check to see if we need to send a
+// notification for this comment if the comment has been recently approved but
+// has not been approved before.
+const commentAcceptedHandleAdapter = (ctx, comment) => {
+  // Don't send a notification for a non-visible comment.
+  if (!comment.visible) {
+    ctx.log.info('comment was not visible, not sending notification');
+    return;
+  }
+
+  // Don't send a notification if the comment was previously visible.
+  if (
+    // TODO: (wyattjoh) this check is quite brittle, replace with a more concrete check.
+    comment.status_history
+      .slice(0, comment.status_history.length - 1)
+      .some(({ type }) => ['ACCEPTED', 'NONE'].includes(type))
+  ) {
+    ctx.log.info(
+      'comment was previously already visible, not sending another notification'
+    );
+    return;
+  }
+
+  // Delegate to the handle function.
+  return handle(ctx, comment);
 };
 
 module.exports = {
@@ -115,5 +140,20 @@ module.exports = {
     },
   },
   translations: path.join(__dirname, 'translations.yml'),
-  notifications: [handler],
+  notifications: [
+    {
+      handle,
+      category: 'reply',
+      event: 'commentAdded',
+      hydrate,
+      digestOrder: 30,
+    },
+    {
+      handle: commentAcceptedHandleAdapter,
+      category: 'reply',
+      event: 'commentAccepted',
+      hydrate,
+      digestOrder: 30,
+    },
+  ],
 };