Push Notifications Fixes - Part 2 (#12092)

* push notifications: handle some more error codes and when the user is loaded using .lean()

* fix lint

* do not send push notification if message is missing

website/client/src/components/shops/quests/index.vue

@@ -301,7 +301,7 @@
                 <h4 class="popover-content-title">{{ item.text }}</h4>
                 <questInfo
                   :quest="item"
-                  :popoverVersion="true"
+                  :popover-version="true"
                 />
               </div>
             </span>

website/server/libs/chat.js

@@ -1,6 +1,6 @@
 import { model as User } from '../models/user'; // eslint-disable-line import/no-cycle
 import { getUserInfo } from './email'; // eslint-disable-line import/no-cycle
-import { sendNotification as sendPushNotification } from './pushNotifications';
+import { sendNotification as sendPushNotification } from './pushNotifications'; // eslint-disable-line import/no-cycle
 
 export async function getAuthorEmailFromMessage (message) {
   const authorId = message.uuid;
@@ -30,6 +30,9 @@ export async function sendChatPushNotifications (user, group, message, mentions,
       if (mentions && mentions.includes(`@${member.auth.local.username}`) && member.preferences.pushNotifications.mentionParty !== false) {
         return;
       }
+
+      if (!message.unformattedText) return;
+
       sendPushNotification(
         member,
         {

website/server/libs/inbox/index.js

@@ -1,6 +1,6 @@
 import { mapInboxMessage, inboxModel as Inbox } from '../../models/message';
 import { getUserInfo, sendTxn as sendTxnEmail } from '../email'; // eslint-disable-line import/no-cycle
-import { sendNotification as sendPushNotification } from '../pushNotifications';
+import { sendNotification as sendPushNotification } from '../pushNotifications'; // eslint-disable-line import/no-cycle
 
 const PM_PER_PAGE = 10;
 
@@ -14,7 +14,7 @@ export async function sentMessage (sender, receiver, message, translate) {
     ]);
   }
 
-  if (receiver.preferences.pushNotifications.newPM !== false) {
+  if (receiver.preferences.pushNotifications.newPM !== false && messageSent.unformattedText) {
     sendPushNotification(
       receiver,
       {

website/server/libs/payments/gems.js

@@ -3,7 +3,7 @@ import { // eslint-disable-line import/no-cycle
   getUserInfo,
   sendTxn as txnEmail,
 } from '../email';
-import { sendNotification as sendPushNotification } from '../pushNotifications';
+import { sendNotification as sendPushNotification } from '../pushNotifications'; // eslint-disable-line import/no-cycle
 import shared from '../../../common';
 
 function getGiftMessage (data, byUsername, gemAmount, language) {

website/server/libs/payments/subscriptions.js

@@ -16,7 +16,7 @@ import {
   NotFound,
 } from '../errors';
 import shared from '../../../common';
-import { sendNotification as sendPushNotification } from '../pushNotifications';
+import { sendNotification as sendPushNotification } from '../pushNotifications'; // eslint-disable-line import/no-cycle
 
 // @TODO: Abstract to shared/constant
 const JOINED_GROUP_PLAN = 'joined group plan';

website/server/libs/pushNotifications.js

@@ -3,6 +3,9 @@ import nconf from 'nconf';
 import apn from 'apn';
 import gcmLib from 'node-gcm'; // works with FCM notifications too
 import logger from './logger';
+import { // eslint-disable-line import/no-cycle
+  model as User,
+} from '../models/user';
 
 const FCM_API_KEY = nconf.get('PUSH_CONFIGS_FCM_SERVER_API_KEY');
 const fcmSender = FCM_API_KEY ? new gcmLib.Sender(FCM_API_KEY) : undefined;
@@ -18,7 +21,7 @@ const apnProvider = APN_ENABLED ? new apn.Provider({
 }) : undefined;
 
 function removePushDevice (user, pushDevice) {
-  return user.update({
+  return User.update({ _id: user._id }, {
     $pull: { pushDevices: { regId: pushDevice.regId } },
   }).exec().catch(err => {
     logger.error(err, `Error removing pushDevice ${pushDevice.regId} for user ${user._id}`);
@@ -80,10 +83,18 @@ export function sendNotification (user, details = {}) {
               // The regId is not valid anymore, remove it
               if (failed === 'NotRegistered') {
                 removePushDevice(user, pushDevice);
-                logger.error(new Error('FCM error, invalid pushDevice'), {
+                logger.error(new Error('FCM error, unregistered pushDevice'), {
-                  response, regId: pushDevice.regId, userId: user._id,
+                  regId: pushDevice.regId, userId: user._id,
                 });
               } else {
+                // An invalid token was registered by mistake
+                // Remove it but log the error differently so that it can be distinguished
+                // from when failed === NotRegistered
+                // Blacklisted can happen in some rare cases,
+                // see https://stackoverflow.com/questions/42136122/why-does-firebase-push-token-return-blacklisted
+                if (failed === 'InvalidRegistration' || pushDevice.regId === 'BLACKLISTED') {
+                  removePushDevice(user, pushDevice);
+                }
                 logger.error(new Error('FCM error'), {
                   response, regId: pushDevice.regId, userId: user._id,
                 });
@@ -110,7 +121,7 @@ export function sendNotification (user, details = {}) {
               // Handle failed push notifications deliveries
               response.failed.forEach(failure => {
                 if (failure.error) { // generic error
-                  logger.error(new Error('APN error'), {
+                  logger.error(new Error('Unhandled APN error'), {
                     response, regId: pushDevice.regId, userId: user._id,
                   });
                 } else { // rejected
@@ -119,10 +130,16 @@ export function sendNotification (user, details = {}) {
                   const { reason } = failure.response;
                   if (reason === 'Unregistered') {
                     removePushDevice(user, pushDevice);
-                    logger.error(new Error('APN error, invalid pushDevice'), {
+                    logger.error(new Error('APN error, unregistered pushDevice'), {
-                      response, regId: pushDevice.regId, userId: user._id,
+                      regId: pushDevice.regId, userId: user._id,
                     });
                   } else {
+                    if (reason === 'BadDeviceToken') {
+                      // An invalid token was registered by mistake
+                      // Remove it but log the error differently so that it can be distinguished
+                      // from when reason === Unregistered
+                      removePushDevice(user, pushDevice);
+                    }
                     logger.error(new Error('APN error'), {
                       response, regId: pushDevice.regId, userId: user._id,
                     });

website/server/models/challenge.js

@@ -11,7 +11,7 @@ import { // eslint-disable-line import/no-cycle
 import { removeFromArray } from '../libs/collectionManipulators';
 import shared from '../../common';
 import { sendTxn as txnEmail } from '../libs/email'; // eslint-disable-line import/no-cycle
-import { sendNotification as sendPushNotification } from '../libs/pushNotifications';
+import { sendNotification as sendPushNotification } from '../libs/pushNotifications'; // eslint-disable-line import/no-cycle
 import { syncableAttrs, setNextDue } from '../libs/taskManager';
 
 const { Schema } = mongoose;

website/server/models/group.js

@@ -28,7 +28,7 @@ import {
 } from '../libs/errors';
 import baseModel from '../libs/baseModel';
 import { sendTxn as sendTxnEmail } from '../libs/email'; // eslint-disable-line import/no-cycle
-import { sendNotification as sendPushNotification } from '../libs/pushNotifications';
+import { sendNotification as sendPushNotification } from '../libs/pushNotifications'; // eslint-disable-line import/no-cycle
 import {
   syncableAttrs,
 } from '../libs/taskManager';
@@ -637,12 +637,15 @@ schema.methods.sendChat = function sendChat (options = {}) {
           return;
         }
       }
-      sendPushNotification(member, {
+
-        identifier: 'chatMention',
+      if (newChatMessage.unformattedText) {
-        title: `${user.profile.name} mentioned you in ${this.name}`,
+        sendPushNotification(member, {
-        message: newChatMessage.unformattedText,
+          identifier: 'chatMention',
-        payload: { type: this.type },
+          title: `${user.profile.name} mentioned you in ${this.name}`,
-      });
+          message: newChatMessage.unformattedText,
+          payload: { type: this.type },
+        });
+      }
     });
   }
   return newChatMessage;