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 => {