Habitica: Create a method to add user notifications through MongoDB's updates

Created on 8 Dec 2016  路  7Comments  路  Source: HabitRPG/habitica

Right now notifications can be added to users through user.addNotification that requires a .save(), we should have an equivalent method that adds notifications using User.update thus not requiring to load and save the entire user

minor status in progress

All 7 comments

I can add this.

@paglias Should the original method be removed when the new one is added?

@khipkin not really, I wanted to add this new method because it'll allow us to add a notification without having to load the user when it's not already loaded

@paglias So when would it be better to use the old method instead of this new one?

@khipkin Technically, it would be better to use the old method when you have the user loaded and you are already making updates to the user. This new method would be used only when you are only adding a notification.

@paglias and @Hus274 I strongly suggest we document these methods and their separate use cases. Honestly, I don't believe many people to pick up on the differences, but if they see examples and the docs - they may learn why.

Doc block example: https://github.com/HabitRPG/habitica/blob/develop/website/server/libs/taskManager.js#L25

@TheHollidayInn Ahh that makes sense! But I agree that documenting the differences would be rather important.

One example would be this PR, where we load the user just to add the notification https://github.com/HabitRPG/habitica/commit/6a63f080adb89ce7afc2c65dc884368cc45ddc71#diff-b950a7c557b35ea58bc2685532c46170R309

I agree that docs are important, also the notification should probably be build using new UserNotification(...) with UserNotification being a model that can be found in models/userNotifications.js to make sure the new notification gets all the data.

Was this page helpful?
0 / 5 - 0 ratings