From 343f276705d994b409c69821381e97f516998209 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 13:30:22 +0100
Subject: [PATCH 1/9] re-enable notifications validation

---
 website/server/models/userNotification.js | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/website/server/models/userNotification.js b/website/server/models/userNotification.js
index cbadd05a3a..d5d160d255 100644
--- a/website/server/models/userNotification.js
+++ b/website/server/models/userNotification.js
@@ -63,15 +63,11 @@ export const schema = new Schema({
     $type: String,
     default: uuid,
     validate: [v => validator.isUUID(v), 'Invalid uuid for userNotification.'],
-    // @TODO: Add these back once we figure out the issue with notifications
-    // See Fix for https://github.com/HabitRPG/habitica/issues/9923
-    // required: true,
+    required: true,
   },
   type: {
     $type: String,
-    // @TODO: Add these back once we figure out the issue with notifications
-    // See Fix for https://github.com/HabitRPG/habitica/issues/9923
-    // required: true,
+    required: true,
     enum: NOTIFICATION_TYPES,
   },
   data: {

From 118e3580d62326df7f2f03a377433714d1142b03 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 20:06:24 +0100
Subject: [PATCH 2/9] refactor notifications cleanup

---
 test/api/unit/models/user.test.js         | 22 +++++++++++++++++-
 website/server/models/user/hooks.js       | 28 +++++++++++++++--------
 website/server/models/userNotification.js | 24 +++++++++----------
 3 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js
index 711c4fd4b6..84a10e80e3 100644
--- a/test/api/unit/models/user.test.js
+++ b/test/api/unit/models/user.test.js
@@ -181,7 +181,7 @@ describe('User Model', () => {
     });
   });
 
-  context('notifications', () => {
+  context.only('notifications', () => {
     it('can add notifications without data', () => {
       const user = new User();
 
@@ -215,6 +215,26 @@ describe('User Model', () => {
       expect(userToJSON.notifications[0].id).to.equal('123');
     });
 
+    it('removes invalid notifications when saving', async () => {
+      const user = new User();
+
+      user.notifications = [
+        null, // invalid, not an object
+        { seen: true }, // invalid, no type or id
+        { id: 123 }, // invalid, no type
+        // invalid, no id, not included here because the id would be added automatically
+        // {type: 'ABC'},
+        { type: 'ABC', id: '123' }, // valid
+      ];
+
+      await user.save();
+      expect(user.notifications.length).to.equal(1);
+
+      expect(user.notifications[0]).to.have.all.keys(['data', 'id', 'type', 'seen']);
+      expect(user.notifications[0].type).to.equal('ABC');
+      expect(user.notifications[0].id).to.equal('123');
+    });
+
     it('can add notifications with data and already marked as seen', () => {
       const user = new User();
 
diff --git a/website/server/models/user/hooks.js b/website/server/models/user/hooks.js
index 5b9e5fe8f2..35c47422f7 100644
--- a/website/server/models/user/hooks.js
+++ b/website/server/models/user/hooks.js
@@ -25,11 +25,6 @@ schema.plugin(baseModel, {
 
     delete plainObj.filters;
 
-    if (originalDoc.notifications) {
-      plainObj.notifications = UserNotification
-        .convertNotificationsToSafeJson(originalDoc.notifications);
-    }
-
     return plainObj;
   },
 });
@@ -182,6 +177,22 @@ function _setProfileName (user) {
   return localUsername || anonymous;
 }
 
+schema.post('init', function postInitUser () {
+  // Cleanup any corrupt data that could have ended up inside the user schema.
+  // In particular:
+  // - tags https://github.com/HabitRPG/habitica/issues/10688
+  // - notifications https://github.com/HabitRPG/habitica/issues/9923
+  // - push devices https://github.com/HabitRPG/habitica/issues/11805
+  //            and https://github.com/HabitRPG/habitica/issues/11868
+
+  // Make sure notifications are loaded
+  if (this.isDirectSelected('notifications')) {
+    this.notifications = UserNotification.cleanupCorruptData(this.notifications);
+  }
+
+  return true;
+});
+
 schema.pre('validate', function preValidateUser (next) {
   // Populate new user with profile name, not running in pre('save') because the field
   // is required and validation fails if it doesn't exists like for new users
@@ -258,11 +269,8 @@ schema.pre('save', true, function preSaveUser (next, done) {
     const unallocatedPointsNotifications = [];
 
     this.notifications = this.notifications.filter(notification => {
-      // Remove corrupt notifications
-      if (!notification || !notification.type) return false;
-
-      // Remove all unsallocated stats points
-      if (notification && notification.type === 'UNALLOCATED_STATS_POINTS') {
+      // Remove all unallocated stats points
+      if (notification.type === 'UNALLOCATED_STATS_POINTS') {
         unallocatedPointsNotifications.push(notification);
         return false;
       }
diff --git a/website/server/models/userNotification.js b/website/server/models/userNotification.js
index d5d160d255..63b550bf15 100644
--- a/website/server/models/userNotification.js
+++ b/website/server/models/userNotification.js
@@ -88,30 +88,30 @@ export const schema = new Schema({
 });
 
 /**
- * Convert notifications to JSON making sure to return only valid data.
- * Fix for https://github.com/HabitRPG/habitica/issues/9923#issuecomment-362869881
- * @TODO Remove once https://github.com/HabitRPG/habitica/issues/9923
- * is fixed
+ * Remove invalid data from an array of notifications.
+ * Fix for https://github.com/HabitRPG/habitica/issues/9923
+ * Called by user's post init hook (models/user/hooks.js)
  */
-schema.statics.convertNotificationsToSafeJson = function convNotifsToSafeJson (notifications) {
+schema.statics.cleanupCorruptData = function cleanupCorruptNotificationsData (notifications) {
   if (!notifications) return notifications;
 
-  let filteredNotifications = notifications.filter(n => {
-    // Exclude notifications with a nullish value
-    if (!n) return false;
-    // Exclude notifications without an id or a type
-    if (!n.id || !n.type) return false;
+  let filteredNotifications = notifications.filter(notification => {
+    // Exclude notifications with a nullish value, no id or no type
+    if (!notification || !notification.id || !notification.type) return false;
     return true;
   });
 
+  // Remove duplicate NEW_CHAT_MESSAGES notifications
+  // can be caused by a race condition when adding a new notification of this type
+  // in group.sendChat if two messages are posted at the same time
   filteredNotifications = _.uniqWith(filteredNotifications, (val, otherVal) => {
-    if (val.type === otherVal.type && val.type === 'NEW_CHAT_MESSAGE') {
+    if (val.type === 'NEW_CHAT_MESSAGE' && val.type === otherVal.type) {
       return val.data.group.id === otherVal.data.group.id;
     }
     return false;
   });
 
-  return filteredNotifications.map(n => n.toJSON());
+  return filteredNotifications;
 };
 
 schema.plugin(baseModel, {

From a11e4d0512bd6bbae8b0fd6fe2b9f2326f2ecc0d Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 20:53:33 +0100
Subject: [PATCH 3/9] fix lint

---
 test/api/unit/models/user.test.js                  | 2 +-
 website/server/controllers/api-v3/notifications.js | 9 +++------
 website/server/middlewares/response.js             | 5 +----
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js
index 84a10e80e3..b637b84753 100644
--- a/test/api/unit/models/user.test.js
+++ b/test/api/unit/models/user.test.js
@@ -181,7 +181,7 @@ describe('User Model', () => {
     });
   });
 
-  context.only('notifications', () => {
+  context('notifications', () => {
     it('can add notifications without data', () => {
       const user = new User();
 
diff --git a/website/server/controllers/api-v3/notifications.js b/website/server/controllers/api-v3/notifications.js
index 16a0410511..8ded24d5eb 100644
--- a/website/server/controllers/api-v3/notifications.js
+++ b/website/server/controllers/api-v3/notifications.js
@@ -5,9 +5,6 @@ import {
 import {
   model as User,
 } from '../../models/user';
-import {
-  model as UserNotification,
-} from '../../models/userNotification';
 
 const api = {};
 
@@ -49,7 +46,7 @@ api.readNotification = {
       $pull: { notifications: { id: req.params.notificationId } },
     }).exec();
 
-    res.respond(200, UserNotification.convertNotificationsToSafeJson(user.notifications));
+    res.respond(200, user.notifications);
   },
 };
 
@@ -92,7 +89,7 @@ api.readNotifications = {
     // See https://github.com/HabitRPG/habitica/pull/9321#issuecomment-354187666 for more info
     user._v += 1;
 
-    res.respond(200, UserNotification.convertNotificationsToSafeJson(user.notifications));
+    res.respond(200, user.notifications);
   },
 };
 
@@ -181,7 +178,7 @@ api.seeNotifications = {
 
     await user.save();
 
-    res.respond(200, UserNotification.convertNotificationsToSafeJson(user.notifications));
+    res.respond(200, user.notifications);
   },
 };
 
diff --git a/website/server/middlewares/response.js b/website/server/middlewares/response.js
index 945cf19dea..3190214cd2 100644
--- a/website/server/middlewares/response.js
+++ b/website/server/middlewares/response.js
@@ -1,7 +1,4 @@
 import packageInfo from '../../../package.json';
-import {
-  model as UserNotification,
-} from '../models/userNotification';
 
 export default function responseHandler (req, res, next) {
   // Only used for successful responses
@@ -16,7 +13,7 @@ export default function responseHandler (req, res, next) {
     if (message) response.message = message;
 
     if (user) {
-      response.notifications = UserNotification.convertNotificationsToSafeJson(user.notifications);
+      response.notifications = user.notifications;
       response.userV = user._v;
     }
 

From 6deee0ffc856ab9f189dc856dd5cb0974d2f1e01 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 21:29:57 +0100
Subject: [PATCH 4/9] notifications tests

---
 test/api/unit/models/user.test.js             | 46 +++++++------------
 test/api/unit/models/userNotification.test.js | 13 +++---
 website/server/models/userNotification.js     |  1 +
 3 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js
index b637b84753..0501e3ed81 100644
--- a/test/api/unit/models/user.test.js
+++ b/test/api/unit/models/user.test.js
@@ -195,17 +195,23 @@ describe('User Model', () => {
       expect(userToJSON.notifications[0].seen).to.eql(false);
     });
 
-    it('removes invalid notifications when calling toJSON', () => {
-      const user = new User();
+    it('removes invalid notifications when loading the user', async () => {
+      let user = new User();
+      await user.save();
+      await user.update({
+        $set: {
+          notifications: [
+            null, // invalid, not an object
+            { seen: true }, // invalid, no type or id
+            { id: 123 }, // invalid, no type
+            // invalid, no id, not included here because the id would be added automatically
+            // {type: 'ABC'},
+            { type: 'ABC', id: '123' }, // valid
+          ],
+        },
+      }).exec();
 
-      user.notifications = [
-        null, // invalid, not an object
-        { seen: true }, // invalid, no type or id
-        { id: 123 }, // invalid, no type
-        // invalid, no id, not included here because the id would be added automatically
-        // {type: 'ABC'},
-        { type: 'ABC', id: '123' }, // valid
-      ];
+      user = await User.findById(user._id).exec();
 
       const userToJSON = user.toJSON();
       expect(userToJSON.notifications.length).to.equal(1);
@@ -215,26 +221,6 @@ describe('User Model', () => {
       expect(userToJSON.notifications[0].id).to.equal('123');
     });
 
-    it('removes invalid notifications when saving', async () => {
-      const user = new User();
-
-      user.notifications = [
-        null, // invalid, not an object
-        { seen: true }, // invalid, no type or id
-        { id: 123 }, // invalid, no type
-        // invalid, no id, not included here because the id would be added automatically
-        // {type: 'ABC'},
-        { type: 'ABC', id: '123' }, // valid
-      ];
-
-      await user.save();
-      expect(user.notifications.length).to.equal(1);
-
-      expect(user.notifications[0]).to.have.all.keys(['data', 'id', 'type', 'seen']);
-      expect(user.notifications[0].type).to.equal('ABC');
-      expect(user.notifications[0].id).to.equal('123');
-    });
-
     it('can add notifications with data and already marked as seen', () => {
       const user = new User();
 
diff --git a/test/api/unit/models/userNotification.test.js b/test/api/unit/models/userNotification.test.js
index c52a44bd21..e9fb8f8c10 100644
--- a/test/api/unit/models/userNotification.test.js
+++ b/test/api/unit/models/userNotification.test.js
@@ -1,7 +1,7 @@
 import { model as UserNotification } from '../../../../website/server/models/userNotification';
 
 describe('UserNotification Model', () => {
-  context('convertNotificationsToSafeJson', () => {
+  context('cleanupCorruptData', () => {
     it('converts an array of notifications to a safe version', () => {
       const notifications = [
         null, // invalid, not an object
@@ -11,11 +11,12 @@ describe('UserNotification Model', () => {
         new UserNotification({ type: 'ABC', id: 123 }), // valid
       ];
 
-      const notificationsToJSON = UserNotification.convertNotificationsToSafeJson(notifications);
-      expect(notificationsToJSON.length).to.equal(1);
-      expect(notificationsToJSON[0]).to.have.all.keys(['data', 'id', 'type', 'seen']);
-      expect(notificationsToJSON[0].type).to.equal('ABC');
-      expect(notificationsToJSON[0].id).to.equal('123');
+      const safeNotifications = UserNotification.cleanupCorruptData(notifications);
+      expect(safeNotifications.length).to.equal(1);
+      expect(safeNotifications[0].data).to.deep.equal({});
+      expect(safeNotifications[0].seen).to.equal(false);
+      expect(safeNotifications[0].type).to.equal('ABC');
+      expect(safeNotifications[0].id).to.equal('123');
     });
   });
 });
diff --git a/website/server/models/userNotification.js b/website/server/models/userNotification.js
index 63b550bf15..b5c6561ed9 100644
--- a/website/server/models/userNotification.js
+++ b/website/server/models/userNotification.js
@@ -93,6 +93,7 @@ export const schema = new Schema({
  * Called by user's post init hook (models/user/hooks.js)
  */
 schema.statics.cleanupCorruptData = function cleanupCorruptNotificationsData (notifications) {
+  console.log('fixing stuff', notifications);
   if (!notifications) return notifications;
 
   let filteredNotifications = notifications.filter(notification => {

From 3f5ee32684680f06d14bbcdc99d5f58c719c2cfd Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 21:49:52 +0100
Subject: [PATCH 5/9] fix invalid push devices

---
 test/api/unit/models/pushDevice.test.js   | 19 ++++++++++
 test/api/unit/models/user.test.js         | 44 ++++++++++++++++++-----
 website/server/models/pushDevice.js       | 27 ++++++++++++++
 website/server/models/user/hooks.js       |  8 +++++
 website/server/models/userNotification.js |  1 -
 5 files changed, 89 insertions(+), 10 deletions(-)
 create mode 100644 test/api/unit/models/pushDevice.test.js

diff --git a/test/api/unit/models/pushDevice.test.js b/test/api/unit/models/pushDevice.test.js
new file mode 100644
index 0000000000..17d34a07b9
--- /dev/null
+++ b/test/api/unit/models/pushDevice.test.js
@@ -0,0 +1,19 @@
+import { model as PushDevice } from '../../../../website/server/models/pushDevice';
+
+describe('PushDevice Model', () => {
+  context('cleanupCorruptData', () => {
+    it('converts an array of push devices to a safe version', () => {
+      const pushDevices = [
+        null, // invalid, not an object
+        { regId: '123' }, // invalid, no type
+        { type: 'android' }, // invalid, no regId
+        new PushDevice({ type: 'android', regId: '1234' }), // valid
+      ];
+
+      const safePushDevices = PushDevice.cleanupCorruptData(pushDevices);
+      expect(safePushDevices.length).to.equal(1);
+      expect(safePushDevices[0].type).to.equal('android');
+      expect(safePushDevices[0].regId).to.equal('1234');
+    });
+  });
+});
diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js
index 0501e3ed81..429cfa1082 100644
--- a/test/api/unit/models/user.test.js
+++ b/test/api/unit/models/user.test.js
@@ -181,18 +181,29 @@ describe('User Model', () => {
     });
   });
 
-  context('notifications', () => {
-    it('can add notifications without data', () => {
-      const user = new User();
+  context('post init', () => {
+    it('removes invalid push devices when loading the user', async () => {
+      let user = new User();
+      await user.save();
+      await user.update({
+        $set: {
+          pushDevices: [
+            null, // invalid, not an object
+            { regId: '123' }, // invalid, no type
+            { type: 'android' }, // invalid, no regId
+            { type: 'android', regId: '1234' }, // valid
+          ],
+        },
+      }).exec();
 
-      user.addNotification('CRON');
+      user = await User.findById(user._id).exec();
 
       const userToJSON = user.toJSON();
-      expect(user.notifications.length).to.equal(1);
-      expect(userToJSON.notifications[0]).to.have.all.keys(['data', 'id', 'type', 'seen']);
-      expect(userToJSON.notifications[0].type).to.equal('CRON');
-      expect(userToJSON.notifications[0].data).to.eql({});
-      expect(userToJSON.notifications[0].seen).to.eql(false);
+      expect(userToJSON.pushDevices.length).to.equal(1);
+
+      expect(userToJSON.pushDevices[0]).to.have.all.keys(['regId', 'type', 'createdAt', 'updatedAt']);
+      expect(userToJSON.pushDevices[0].type).to.equal('android');
+      expect(userToJSON.pushDevices[0].regId).to.equal('1234');
     });
 
     it('removes invalid notifications when loading the user', async () => {
@@ -220,6 +231,21 @@ describe('User Model', () => {
       expect(userToJSON.notifications[0].type).to.equal('ABC');
       expect(userToJSON.notifications[0].id).to.equal('123');
     });
+  });
+
+  context('notifications', () => {
+    it('can add notifications without data', () => {
+      const user = new User();
+
+      user.addNotification('CRON');
+
+      const userToJSON = user.toJSON();
+      expect(user.notifications.length).to.equal(1);
+      expect(userToJSON.notifications[0]).to.have.all.keys(['data', 'id', 'type', 'seen']);
+      expect(userToJSON.notifications[0].type).to.equal('CRON');
+      expect(userToJSON.notifications[0].data).to.eql({});
+      expect(userToJSON.notifications[0].seen).to.eql(false);
+    });
 
     it('can add notifications with data and already marked as seen', () => {
       const user = new User();
diff --git a/website/server/models/pushDevice.js b/website/server/models/pushDevice.js
index b84a16aac4..0fc4bd3f69 100644
--- a/website/server/models/pushDevice.js
+++ b/website/server/models/pushDevice.js
@@ -1,4 +1,5 @@
 import mongoose from 'mongoose';
+import _ from 'lodash';
 import baseModel from '../libs/baseModel';
 
 const { Schema } = mongoose;
@@ -19,4 +20,30 @@ schema.plugin(baseModel, {
   _id: false,
 });
 
+/**
+ * Remove invalid data from an array of push devices.
+ * Fix for https://github.com/HabitRPG/habitica/issues/11805
+ * and https://github.com/HabitRPG/habitica/issues/11868
+ * Called by user's post init hook (models/user/hooks.js)
+ */
+schema.statics.cleanupCorruptData = function cleanupCorruptPushDevicesData (pushDevices) {
+  if (!pushDevices) return pushDevices;
+
+  let filteredPushDevices = pushDevices.filter(pushDevice => {
+    // Exclude push devices with a nullish value, no id or no type
+    if (!pushDevice || !pushDevice.regId || !pushDevice.type) return false;
+    return true;
+  });
+
+  // Remove duplicate push devices
+  // can be caused by a race condition when adding a new push device
+  filteredPushDevices = _.uniqWith(filteredPushDevices, (val, otherVal) => {
+    if (val.regId === otherVal.regId && val.type === otherVal.type) return true;
+    return false;
+  });
+
+  return filteredPushDevices;
+};
+
+
 export const model = mongoose.model('PushDevice', schema);
diff --git a/website/server/models/user/hooks.js b/website/server/models/user/hooks.js
index 35c47422f7..2d8dcd9e78 100644
--- a/website/server/models/user/hooks.js
+++ b/website/server/models/user/hooks.js
@@ -6,6 +6,9 @@ import * as Tasks from '../task';
 import {
   model as UserNotification,
 } from '../userNotification';
+import {
+  model as PushDevice,
+} from '../pushDevice';
 import { // eslint-disable-line import/no-cycle
   userActivityWebhook,
 } from '../../libs/webhook';
@@ -190,6 +193,11 @@ schema.post('init', function postInitUser () {
     this.notifications = UserNotification.cleanupCorruptData(this.notifications);
   }
 
+  // Make sure pushDevices are loaded
+  if (this.isDirectSelected('pushDevices')) {
+    this.pushDevices = PushDevice.cleanupCorruptData(this.pushDevices);
+  }
+
   return true;
 });
 
diff --git a/website/server/models/userNotification.js b/website/server/models/userNotification.js
index b5c6561ed9..63b550bf15 100644
--- a/website/server/models/userNotification.js
+++ b/website/server/models/userNotification.js
@@ -93,7 +93,6 @@ export const schema = new Schema({
  * Called by user's post init hook (models/user/hooks.js)
  */
 schema.statics.cleanupCorruptData = function cleanupCorruptNotificationsData (notifications) {
-  console.log('fixing stuff', notifications);
   if (!notifications) return notifications;
 
   let filteredNotifications = notifications.filter(notification => {

From 979d0c519d8587963f35b562d62749c0a68c5af7 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 22:10:11 +0100
Subject: [PATCH 6/9] add final tests for push devices and notifications

---
 test/api/unit/models/pushDevice.test.js       | 14 ++++
 test/api/unit/models/user.test.js             | 64 +++++++++++++++++++
 test/api/unit/models/userNotification.test.js | 32 ++++++++++
 3 files changed, 110 insertions(+)

diff --git a/test/api/unit/models/pushDevice.test.js b/test/api/unit/models/pushDevice.test.js
index 17d34a07b9..6077a75113 100644
--- a/test/api/unit/models/pushDevice.test.js
+++ b/test/api/unit/models/pushDevice.test.js
@@ -15,5 +15,19 @@ describe('PushDevice Model', () => {
       expect(safePushDevices[0].type).to.equal('android');
       expect(safePushDevices[0].regId).to.equal('1234');
     });
+
+    it('removes duplicates', () => {
+      const pushDevices = [
+        new PushDevice({ type: 'android', regId: '1234' }),
+        new PushDevice({ type: 'android', regId: '1234' }),
+        new PushDevice({ type: 'iphone', regId: '1234' }), // not duplicate
+        new PushDevice({ type: 'android', regId: '12345' }), // not duplicate
+      ];
+
+      const safePushDevices = PushDevice.cleanupCorruptData(pushDevices);
+      expect(safePushDevices.length).to.equal(3);
+      expect(safePushDevices[0].type).to.equal('android');
+      expect(safePushDevices[0].regId).to.equal('1234');
+    });
   });
 });
diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js
index 429cfa1082..9425fee275 100644
--- a/test/api/unit/models/user.test.js
+++ b/test/api/unit/models/user.test.js
@@ -206,6 +206,28 @@ describe('User Model', () => {
       expect(userToJSON.pushDevices[0].regId).to.equal('1234');
     });
 
+    it('removes duplicate push devices when loading the user', async () => {
+      let user = new User();
+      await user.save();
+      await user.update({
+        $set: {
+          pushDevices: [
+            { type: 'android', regId: '1234' },
+            { type: 'android', regId: '1234' },
+          ],
+        },
+      }).exec();
+
+      user = await User.findById(user._id).exec();
+
+      const userToJSON = user.toJSON();
+      expect(userToJSON.pushDevices.length).to.equal(1);
+
+      expect(userToJSON.pushDevices[0]).to.have.all.keys(['regId', 'type', 'createdAt', 'updatedAt']);
+      expect(userToJSON.pushDevices[0].type).to.equal('android');
+      expect(userToJSON.pushDevices[0].regId).to.equal('1234');
+    });
+
     it('removes invalid notifications when loading the user', async () => {
       let user = new User();
       await user.save();
@@ -231,6 +253,48 @@ describe('User Model', () => {
       expect(userToJSON.notifications[0].type).to.equal('ABC');
       expect(userToJSON.notifications[0].id).to.equal('123');
     });
+
+    it('removes multiple NEW_CHAT_MESSAGE for the same group', async () => {
+      let user = new User();
+      await user.save();
+      await user.update({
+        $set: {
+          notifications: [
+            {
+              type: 'NEW_CHAT_MESSAGE',
+              id: 123,
+              data: { group: { id: 12345 } },
+            },
+            {
+              type: 'NEW_CHAT_MESSAGE',
+              id: 1234,
+              data: { group: { id: 12345 } },
+            },
+            {
+              type: 'NEW_CHAT_MESSAGE',
+              id: 123,
+              data: { group: { id: 123456 } },
+            }, // not duplicate, different group
+            {
+              type: 'NEW_CHAT_MESSAGE_DIFF',
+              id: 123,
+              data: { group: { id: 12345 } },
+            }, // not duplicate, different type
+          ],
+        },
+      }).exec();
+
+      user = await User.findById(user._id).exec();
+
+      const userToJSON = user.toJSON();
+      expect(userToJSON.notifications.length).to.equal(3);
+
+      expect(userToJSON.notifications[0]).to.have.all.keys(['data', 'id', 'type', 'seen']);
+      expect(userToJSON.notifications[0].type).to.equal('NEW_CHAT_MESSAGE');
+      expect(userToJSON.notifications[0].id).to.equal('123');
+      expect(userToJSON.notifications[0].data).to.deep.equal({ group: { id: 12345 } });
+      expect(userToJSON.notifications[0].seen).to.equal(false);
+    });
   });
 
   context('notifications', () => {
diff --git a/test/api/unit/models/userNotification.test.js b/test/api/unit/models/userNotification.test.js
index e9fb8f8c10..772e8dba8f 100644
--- a/test/api/unit/models/userNotification.test.js
+++ b/test/api/unit/models/userNotification.test.js
@@ -18,5 +18,37 @@ describe('UserNotification Model', () => {
       expect(safeNotifications[0].type).to.equal('ABC');
       expect(safeNotifications[0].id).to.equal('123');
     });
+
+    it('removes multiple NEW_CHAT_MESSAGE for the same group', () => {
+      const notifications = [
+        new UserNotification({
+          type: 'NEW_CHAT_MESSAGE',
+          id: 123,
+          data: { group: { id: 12345 } },
+        }),
+        new UserNotification({
+          type: 'NEW_CHAT_MESSAGE',
+          id: 1234,
+          data: { group: { id: 12345 } },
+        }),
+        new UserNotification({
+          type: 'NEW_CHAT_MESSAGE',
+          id: 123,
+          data: { group: { id: 123456 } },
+        }), // not duplicate, different group
+        new UserNotification({
+          type: 'NEW_CHAT_MESSAGE_DIFF',
+          id: 123,
+          data: { group: { id: 12345 } },
+        }), // not duplicate, different type
+      ];
+
+      const safeNotifications = UserNotification.cleanupCorruptData(notifications);
+      expect(safeNotifications.length).to.equal(3);
+      expect(safeNotifications[0].data).to.deep.equal({ group: { id: 12345 } });
+      expect(safeNotifications[0].seen).to.equal(false);
+      expect(safeNotifications[0].type).to.equal('NEW_CHAT_MESSAGE');
+      expect(safeNotifications[0].id).to.equal('123');
+    });
   });
 });

From c5208f0ef62550468bf9571b87594bdfe5c51f77 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 22:21:53 +0100
Subject: [PATCH 7/9] fix tags

---
 test/api/unit/models/tag.test.js    | 19 +++++++++++++++++++
 test/api/unit/models/user.test.js   | 25 +++++++++++++++++++++++++
 website/server/models/tag.js        | 15 +++++++++++++++
 website/server/models/user/hooks.js |  8 ++++++++
 4 files changed, 67 insertions(+)
 create mode 100644 test/api/unit/models/tag.test.js

diff --git a/test/api/unit/models/tag.test.js b/test/api/unit/models/tag.test.js
new file mode 100644
index 0000000000..1585dd11ad
--- /dev/null
+++ b/test/api/unit/models/tag.test.js
@@ -0,0 +1,19 @@
+import { model as Tag } from '../../../../website/server/models/tag';
+
+describe('Tag Model', () => {
+  context('cleanupCorruptData', () => {
+    it('converts an array of tags to a safe version', () => {
+      const tags = [
+        null, // invalid, not an object
+        { name: '123' }, // invalid, no id
+        { id: '123' }, // invalid, no name
+        new Tag({ name: 'ABC', id: 123 }), // valid
+      ];
+
+      const safetags = Tag.cleanupCorruptData(tags);
+      expect(safetags.length).to.equal(1);
+      expect(safetags[0].name).to.equal('ABC');
+      expect(safetags[0].id).to.equal('123');
+    });
+  });
+});
diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js
index 9425fee275..fd2a3f4061 100644
--- a/test/api/unit/models/user.test.js
+++ b/test/api/unit/models/user.test.js
@@ -182,6 +182,31 @@ describe('User Model', () => {
   });
 
   context('post init', () => {
+    it('removes invalid tags when loading the user', async () => {
+      let user = new User();
+      await user.save();
+      await user.update({
+        $set: {
+          tags: [
+            null, // invalid, not an object
+            // { name: '123' }, // invalid, no id - generated automatically
+            { id: '123' }, // invalid, no name
+            { name: 'ABC', id: '1234' }, // valid
+          ],
+        },
+      }).exec();
+
+      user = await User.findById(user._id).exec();
+
+      const userToJSON = user.toJSON();
+      console.log(userToJSON.tags);
+      expect(userToJSON.tags.length).to.equal(1);
+
+      expect(userToJSON.tags[0]).to.have.all.keys(['id', 'name']);
+      expect(userToJSON.tags[0].id).to.equal('1234');
+      expect(userToJSON.tags[0].name).to.equal('ABC');
+    });
+
     it('removes invalid push devices when loading the user', async () => {
       let user = new User();
       await user.save();
diff --git a/website/server/models/tag.js b/website/server/models/tag.js
index 1fa8bc59a7..94afd95bd8 100644
--- a/website/server/models/tag.js
+++ b/website/server/models/tag.js
@@ -33,4 +33,19 @@ schema.statics.sanitizeUpdate = function sanitizeUpdate (updateObj) {
   return this.sanitize(updateObj, noUpdate);
 };
 
+/**
+ * Remove invalid data from an array of tags.
+ * Fix for https://github.com/HabitRPG/habitica/issues/10688
+ * Called by user's post init hook (models/user/hooks.js)
+ */
+schema.statics.cleanupCorruptData = function cleanupCorruptTagsData (tags) {
+  if (!tags) return tags;
+
+  return tags.filter(tag => {
+    // Exclude tags with a nullish value or no id
+    if (!tag || !tag.id || !tag.name) return false;
+    return true;
+  });
+};
+
 export const model = mongoose.model('Tag', schema);
diff --git a/website/server/models/user/hooks.js b/website/server/models/user/hooks.js
index 2d8dcd9e78..63aa42ed57 100644
--- a/website/server/models/user/hooks.js
+++ b/website/server/models/user/hooks.js
@@ -9,6 +9,9 @@ import {
 import {
   model as PushDevice,
 } from '../pushDevice';
+import {
+  model as Tag,
+} from '../tag';
 import { // eslint-disable-line import/no-cycle
   userActivityWebhook,
 } from '../../libs/webhook';
@@ -198,6 +201,11 @@ schema.post('init', function postInitUser () {
     this.pushDevices = PushDevice.cleanupCorruptData(this.pushDevices);
   }
 
+  // Make sure tags are loaded
+  if (this.isDirectSelected('tags')) {
+    this.tags = Tag.cleanupCorruptData(this.tags);
+  }
+
   return true;
 });
 

From c2a79e1d7c6905c0ea33225ef92a31b88f7dc3be Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 1 Mar 2020 22:37:29 +0100
Subject: [PATCH 8/9] fix tests

---
 test/api/unit/middlewares/response.js | 41 +++------------------------
 1 file changed, 4 insertions(+), 37 deletions(-)

diff --git a/test/api/unit/middlewares/response.js b/test/api/unit/middlewares/response.js
index b8157287be..dbc689276a 100644
--- a/test/api/unit/middlewares/response.js
+++ b/test/api/unit/middlewares/response.js
@@ -34,7 +34,7 @@ describe('response middleware', () => {
     expect(res.json).to.be.calledWith({
       success: true,
       data: { field: 1 },
-      notifications: [],
+      notifications: res.locals.user.notifications,
       userV: res.locals.user._v,
       appVersion: packageInfo.version,
     });
@@ -52,7 +52,7 @@ describe('response middleware', () => {
       success: true,
       data: { field: 1 },
       message: 'hello',
-      notifications: [],
+      notifications: res.locals.user.notifications,
       userV: res.locals.user._v,
       appVersion: packageInfo.version,
     });
@@ -69,7 +69,7 @@ describe('response middleware', () => {
     expect(res.json).to.be.calledWith({
       success: false,
       data: { field: 1 },
-      notifications: [],
+      notifications: res.locals.user.notifications,
       userV: res.locals.user._v,
       appVersion: packageInfo.version,
     });
@@ -84,42 +84,9 @@ describe('response middleware', () => {
     expect(res.json).to.be.calledWith({
       success: true,
       data: { field: 1 },
-      notifications: [],
+      notifications: res.locals.user.notifications,
       userV: 0,
       appVersion: packageInfo.version,
     });
   });
-
-  it('returns notifications if a user is authenticated', () => {
-    const { user } = res.locals;
-
-    user.notifications = [
-      null, // invalid, not an object
-      { seen: true }, // invalid, no type or id
-      { id: 123 }, // invalid, no type
-      // invalid, no id, not included here because the id would be added automatically
-      // {type: 'ABC'},
-      { type: 'ABC', id: '123' }, // valid
-    ];
-
-    responseMiddleware(req, res, next);
-    res.respond(200, { field: 1 });
-
-    expect(res.json).to.be.calledOnce;
-
-    expect(res.json).to.be.calledWith({
-      success: true,
-      data: { field: 1 },
-      notifications: [
-        {
-          type: 'ABC',
-          id: '123',
-          data: {},
-          seen: false,
-        },
-      ],
-      userV: res.locals.user._v,
-      appVersion: packageInfo.version,
-    });
-  });
 });

From 90f88c42f6e8d28d942df68e23bdf8743042126d Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Mon, 2 Mar 2020 10:26:21 +0100
Subject: [PATCH 9/9] remove console.log

---
 test/api/unit/models/user.test.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/api/unit/models/user.test.js b/test/api/unit/models/user.test.js
index fd2a3f4061..49ec464c96 100644
--- a/test/api/unit/models/user.test.js
+++ b/test/api/unit/models/user.test.js
@@ -199,7 +199,6 @@ describe('User Model', () => {
       user = await User.findById(user._id).exec();
 
       const userToJSON = user.toJSON();
-      console.log(userToJSON.tags);
       expect(userToJSON.tags.length).to.equal(1);
 
       expect(userToJSON.tags[0]).to.have.all.keys(['id', 'name']);