Remove user wrapping (#9960)

* remove user wrapping, fixes #9146

* update tests

* fix tests

test/api/v3/integration/chat/POST-chat_seen.test.js

@@ -63,6 +63,8 @@ describe('POST /groups/:id/chat/seen', () => {
       const initialNotifications = partyMember.notifications.length;
       await partyMember.post(`/groups/${party._id}/chat/seen`);
 
+      await sleep(0.5);
+
       let partyMemberThatHasSeenChat = await partyMember.get('/user');
 
       expect(partyMemberThatHasSeenChat.notifications.length).to.equal(initialNotifications - 1);

test/api/v3/unit/libs/cron.test.js

@@ -32,10 +32,6 @@ describe('cron', () => {
     });
 
     sinon.spy(analytics, 'track');
-
-    user._statsComputed = {
-      mp: 10,
-    };
   });
 
   afterEach(() => {
@@ -444,9 +440,13 @@ describe('cron', () => {
       tasksByType.dailys = [];
       tasksByType.dailys.push(task);
 
-      user._statsComputed = {
+      const statsComputedRes = common.statsComputed(user);
-        con: 1,
+      const stubbedStatsComputed = sinon.stub(common, 'statsComputed');
-      };
+      stubbedStatsComputed.returns(Object.assign(statsComputedRes, {con: 1}));
+    });
+
+    afterEach(() => {
+      common.statsComputed.restore();
     });
 
     it('computes isDue', () => {
@@ -855,9 +855,13 @@ describe('cron', () => {
       tasksByType.dailys = [];
       tasksByType.dailys.push(task);
 
-      user._statsComputed = {
+      const statsComputedRes = common.statsComputed(user);
-        con: 1,
+      const stubbedStatsComputed = sinon.stub(common, 'statsComputed');
-      };
+      stubbedStatsComputed.returns(Object.assign(statsComputedRes, {con: 1}));
+    });
+
+    afterEach(() => {
+      common.statsComputed.restore();
     });
 
     it('stores a new entry in user.history.exp', () => {
@@ -972,18 +976,27 @@ describe('cron', () => {
 
   describe('adding mp', () => {
     it('should add mp to user', () => {
+      const statsComputedRes = common.statsComputed(user);
+      const stubbedStatsComputed = sinon.stub(common, 'statsComputed');
+
       let mpBefore = user.stats.mp;
       tasksByType.dailys[0].completed = true;
-      user._statsComputed.maxMP = 100;
+      stubbedStatsComputed.returns(Object.assign(statsComputedRes, {maxMP: 100}));
       cron({user, tasksByType, daysMissed, analytics});
       expect(user.stats.mp).to.be.greaterThan(mpBefore);
+
+      common.statsComputed.restore();
     });
 
-    it('set user\'s mp to user._statsComputed.maxMP when user.stats.mp is greater', () => {
+    it('set user\'s mp to statsComputed.maxMP when user.stats.mp is greater', () => {
+      const statsComputedRes = common.statsComputed(user);
+      const stubbedStatsComputed = sinon.stub(common, 'statsComputed');
       user.stats.mp = 120;
-      user._statsComputed.maxMP = 100;
+      stubbedStatsComputed.returns(Object.assign(statsComputedRes, {maxMP: 100}));
       cron({user, tasksByType, daysMissed, analytics});
-      expect(user.stats.mp).to.equal(user._statsComputed.maxMP);
+      expect(user.stats.mp).to.equal(common.statsComputed(user).maxMP);
+
+      common.statsComputed.restore();
     });
   });
 
@@ -998,14 +1011,18 @@ describe('cron', () => {
       tasksByType.dailys = [];
       tasksByType.dailys.push(task);
 
-      user._statsComputed = {
+      const statsComputedRes = common.statsComputed(user);
-        con: 1,
+      const stubbedStatsComputed = sinon.stub(common, 'statsComputed');
-      };
+      stubbedStatsComputed.returns(Object.assign(statsComputedRes, {con: 1}));
 
       daysMissed = 1;
       tasksByType.dailys[0].startDate = moment(new Date()).subtract({days: 1});
     });
 
+    afterEach(() => {
+      common.statsComputed.restore();
+    });
+
     it('resets user progress', () => {
       cron({user, tasksByType, daysMissed, analytics});
       expect(user.party.quest.progress.up).to.equal(0);
@@ -1023,7 +1040,10 @@ describe('cron', () => {
     it('adds a user notification', () => {
       let mpBefore = user.stats.mp;
       tasksByType.dailys[0].completed = true;
-      user._statsComputed.maxMP = 100;
+
+      const statsComputedRes = common.statsComputed(user);
+      const stubbedStatsComputed = sinon.stub(common, 'statsComputed');
+      stubbedStatsComputed.returns(Object.assign(statsComputedRes, {maxMP: 100}));
 
       daysMissed = 1;
       let hpBefore = user.stats.hp;
@@ -1037,12 +1057,17 @@ describe('cron', () => {
         hp: user.stats.hp - hpBefore,
         mp: user.stats.mp - mpBefore,
       });
+
+      common.statsComputed.restore();
     });
 
     it('condenses multiple notifications into one', () => {
       let mpBefore1 = user.stats.mp;
       tasksByType.dailys[0].completed = true;
-      user._statsComputed.maxMP = 100;
+
+      const statsComputedRes = common.statsComputed(user);
+      const stubbedStatsComputed = sinon.stub(common, 'statsComputed');
+      stubbedStatsComputed.returns(Object.assign(statsComputedRes, {maxMP: 100}));
 
       daysMissed = 1;
       let hpBefore1 = user.stats.hp;
@@ -1073,6 +1098,7 @@ describe('cron', () => {
         mp: user.stats.mp - mpBefore2 - (mpBefore2 - mpBefore1),
       });
       expect(user.notifications[0].type).to.not.equal('CRON');
+      common.statsComputed.restore();
     });
   });
 

test/api/v3/unit/middlewares/cronMiddleware.js

@@ -37,11 +37,6 @@ describe('cron middleware', () => {
 
     user.save()
     .then(savedUser => {
-      savedUser._statsComputed = {
-        mp: 10,
-        maxMP: 100,
-      };
-
       res.locals.user = savedUser;
       res.analytics = analyticsService;
       done();

test/common/fns/statsComputed.test.js

@@ -10,14 +10,6 @@ describe('common.fns.statsComputed', () => {
     user = generateUser();
   });
 
-  it('returns the same result if called directly, through user.fns.statsComputed, or user._statsComputed', () => {
-    let result = statsComputed(user);
-    let result2 = user._statsComputed;
-    let result3 = user.fns.statsComputed();
-    expect(result).to.eql(result2);
-    expect(result).to.eql(result3);
-  });
-
   it('returns default values', () => {
     let result = statsComputed(user);
     expect(result.per).to.eql(0);

test/common/ops/hourglassPurchase.js

@@ -9,7 +9,7 @@ import {
   generateUser,
 } from '../../helpers/common.helper';
 
-describe('user.ops.hourglassPurchase', () => {
+describe('common.ops.hourglassPurchase', () => {
   let user;
 
   beforeEach(() => {

test/common/ops/scoreTask.test.js

@@ -7,7 +7,6 @@ import {
   generateTodo,
   generateReward,
 } from '../../helpers/common.helper';
-import common from '../../../website/common';
 import i18n from '../../../website/common/script/i18n';
 import {
   NotAuthorized,
@@ -16,17 +15,9 @@ import crit from '../../../website/common/script/fns/crit';
 
 let EPSILON = 0.0001; // negligible distance between datapoints
 
-/* Helper Functions */
-let rewrapUser = (user) => {
-  user._wrapped = false;
-  common.wrap(user);
-  return user;
-};
-
 let beforeAfter = () => {
   let beforeUser = generateUser();
   let afterUser = _.cloneDeep(beforeUser);
-  rewrapUser(afterUser);
 
   return {
     beforeUser,

test/helpers/common.helper.js

@@ -1,6 +1,5 @@
 import mongoose from 'mongoose';
 
-import { wrap as wrapUser } from '../../website/common/script/index';
 import { model as User } from '../../website/server/models/user';
 import {
   DailySchema,
@@ -13,8 +12,6 @@ export {translate} from './translate';
 export function generateUser (options = {}) {
   let user = new User(options).toObject();
 
-  wrapUser(user);
-
   return user;
 }
 

website/client/store/actions/members.js

@@ -126,7 +126,6 @@ export async function removeMember (store, payload) {
 // }
 //
 // function _prepareMember(member, self) {
-//   Shared.wrap(member, false);
 //   self.selectedMember = members[member._id];
 // }
 //

website/common/browserify.js

@@ -1,9 +0,0 @@
-require('babel-polyfill');
-
-var shared = require('./script/index');
-var _ = require('lodash');
-var moment = require('moment');
-
-window.habitrpgShared = shared;
-window._ = _;
-window.moment = moment;

website/common/script/index.js

@@ -1,5 +1,3 @@
-import partial from 'lodash/partial';
-
 // When using a common module from the website or the server NEVER import the module directly
 // but access it through `api` (the main common) module, otherwise you would require the non transpiled version of the file in production.
 let api = module.exports = {};
@@ -212,121 +210,3 @@ api.ops = {
   markPmsRead,
   pinnedGearUtils,
 };
-
-/*
-------------------------------------------------------
-User (prototype wrapper to give it ops, helper funcs, and virtuals
-------------------------------------------------------
- */
-
-/*
-User is now wrapped (both on client and server), adding a few new properties:
-  * getters (_statsComputed)
-  * user.fns, which is a bunch of helper functions
-    These were originally up above, but they make more sense belonging to the user object so we don't have to pass
-    the user object all over the place. In fact, we should pull in more functions such as cron(), updateStats(), etc.
-  * user.ops, which is super important:
-
-If a function is inside user.ops, it has magical properties. If you call it on the client it updates the user object in
-the browser and when it's done it automatically POSTs to the server, calling src/controllers/user.js#OP_NAME (the exact same name
-of the op function). The first argument req is {query, body, params}, it's what the express controller function
-expects. This means we call our functions as if we were calling an Express route. Eg, instead of score(task, direction),
-we call score({params:{id:task.id,direction:direction}}). This also forces us to think about our routes (whether to use
-params, query, or body for variables). see http://stackoverflow.com/questions/4024271/rest-api-best-practices-where-to-put-parameters
-
-If `src/controllers/user.js#OP_NAME` doesn't exist on the server, it's automatically added. It runs the code in user.ops.OP_NAME
-to update the user model server-side, then performs `user.save()`. You can see this in action for `user.ops.buy`. That
-function doesn't exist on the server - so the client calls it, it updates user in the browser, auto-POSTs to server, server
-handles it by calling `user.ops.buy` again (to update user on the server), and then saves. We can do this for
-everything that doesn't need any code difference from what's in user.ops.OP_NAME for special-handling server-side. If we
-*do* need special handling, just add `src/controllers/user.js#OP_NAME` to override the user.ops.OP_NAME, and be
-sure to call user.ops.OP_NAME at some point within the overridden function.
-
-TODO
-  * Is this the best way to wrap the user object? I thought of using user.prototype, but user is an object not a Function.
-    user on the server is a Mongoose model, so we can use prototype - but to do it on the client, we'd probably have to
-    move to $resource for user
-  * Move to $resource!
- */
-
-import importedOps from './ops';
-import importedFns from './fns';
-
-// TODO Kept for the client side
-api.wrap = function wrapUser (user, main = true) {
-  if (user._wrapped) return;
-  user._wrapped = true;
-
-  // Make markModified available on the client side as a noop function
-  if (!user.markModified) {
-    user.markModified = function noopMarkModified () {};
-  }
-
-  // same for addNotification
-  if (!user.addNotification) {
-    user.addNotification = function noopAddNotification () {};
-  }
-
-  if (main) {
-    user.ops = {
-      sleep: partial(importedOps.sleep, user),
-      revive: partial(importedOps.revive, user),
-      reset: partial(importedOps.reset, user),
-      reroll: partial(importedOps.reroll, user),
-      rebirth: partial(importedOps.rebirth, user),
-      allocateNow: partial(importedOps.allocateNow, user),
-      sortTask: partial(importedOps.sortTask, user),
-      updateTask: partial(importedOps.updateTask, user),
-      deleteTask: partial(importedOps.deleteTask, user),
-      addTask: partial(importedOps.addTask, user),
-      addTag: partial(importedOps.addTag, user),
-      sortTag: partial(importedOps.sortTag, user),
-      updateTag: partial(importedOps.updateTag, user),
-      deleteTag: partial(importedOps.deleteTag, user),
-      clearPMs: partial(importedOps.clearPMs, user),
-      deletePM: partial(importedOps.deletePM, user),
-      blockUser: partial(importedOps.blockUser, user),
-      feed: partial(importedOps.feed, user),
-      buySpecialSpell: partial(importedOps.buySpecialSpell, user),
-      purchase: partial(importedOps.purchase, user),
-      releasePets: partial(importedOps.releasePets, user),
-      releaseMounts: partial(importedOps.releaseMounts, user),
-      releaseBoth: partial(importedOps.releaseBoth, user),
-      buy: partial(importedOps.buy, user),
-      buyHealthPotion: partial(importedOps.buyHealthPotion, user),
-      buyArmoire: partial(importedOps.buyArmoire, user),
-      buyGear: partial(importedOps.buyGear, user),
-      buyQuest: partial(importedOps.buyQuest, user),
-      buyMysterySet: partial(importedOps.buyMysterySet, user),
-      hourglassPurchase: partial(importedOps.hourglassPurchase, user),
-      sell: partial(importedOps.sell, user),
-      equip: partial(importedOps.equip, user),
-      hatch: partial(importedOps.hatch, user),
-      unlock: partial(importedOps.unlock, user),
-      changeClass: partial(importedOps.changeClass, user),
-      disableClasses: partial(importedOps.disableClasses, user),
-      allocate: partial(importedOps.allocate, user),
-      readCard: partial(importedOps.readCard, user),
-      openMysteryItem: partial(importedOps.openMysteryItem, user),
-      score: partial(importedOps.scoreTask, user),
-      markPmsRead: partial(importedOps.markPmsRead, user),
-    };
-  }
-
-  user.fns = {
-    handleTwoHanded: partial(importedFns.handleTwoHanded, user),
-    predictableRandom: partial(importedFns.predictableRandom, user),
-    crit: partial(importedFns.crit.crit, user),
-    randomDrop: partial(importedFns.randomDrop, user),
-    autoAllocate: partial(importedFns.autoAllocate, user),
-    updateStats: partial(importedFns.updateStats, user),
-    statsComputed: partial(statsComputed, user),
-    ultimateGear: partial(importedFns.ultimateGear, user),
-  };
-
-  Object.defineProperty(user, '_statsComputed', {
-    get () {
-      return statsComputed(user);
-    },
-  });
-};

website/server/controllers/api-v3/tasks.js

@@ -607,7 +607,7 @@ api.scoreTask = {
 
     let [delta] = common.ops.scoreTask({task, user, direction}, req);
     // Drop system (don't run on the client, as it would only be discarded since ops are sent to the API, not the results)
-    if (direction === 'up') user.fns.randomDrop({task, delta}, req, res.analytics);
+    if (direction === 'up') common.fns.randomDrop(user, {task, delta}, req, res.analytics);
 
     // If a todo was completed or uncompleted move it in or out of the user.tasksOrder.todos list
     // TODO move to common code?

website/server/controllers/api-v3/user.js

@@ -470,7 +470,7 @@ api.getUserAnonymized = {
     let user = res.locals.user.toJSON();
     user.stats.toNextLevel = common.tnl(user.stats.lvl);
     user.stats.maxHealth = common.maxHealth;
-    user.stats.maxMP = res.locals.user._statsComputed.maxMP;
+    user.stats.maxMP = common.statsComputed(res.locals.user).maxMP;
 
     delete user.apiToken;
     if (user.auth) {

website/server/libs/cron.js

@@ -428,8 +428,8 @@ export function cron (options = {}) {
   // Add 10 MP, or 10% of max MP if that'd be more. Perform this after Perfect Day for maximum benefit
   // Adjust for fraction of dailies completed
   if (dailyDueUnchecked === 0 && dailyChecked === 0) dailyChecked = 1;
-  user.stats.mp += _.max([10, 0.1 * user._statsComputed.maxMP]) * dailyChecked / (dailyDueUnchecked + dailyChecked);
+  user.stats.mp += _.max([10, 0.1 * common.statsComputed(user).maxMP]) * dailyChecked / (dailyDueUnchecked + dailyChecked);
-  if (user.stats.mp > user._statsComputed.maxMP) user.stats.mp = user._statsComputed.maxMP;
+  if (user.stats.mp > common.statsComputed(user).maxMP) user.stats.mp = common.statsComputed(user).maxMP;
 
   // After all is said and done, progress up user's effect on quest, return those values & reset the user's
   let progress = user.party.quest.progress;

website/server/models/user/hooks.js

@@ -1,4 +1,4 @@
-import shared from '../../../common';
+import common from '../../../common';
 import _ from 'lodash';
 import moment from 'moment';
 import Bluebird from 'bluebird';
@@ -26,10 +26,6 @@ schema.plugin(baseModel, {
   },
 });
 
-schema.post('init', function postInitUser (doc) {
-  shared.wrap(doc);
-});
-
 function findTag (user, tagName) {
   let tagID = _.find(user.tags, (userTag) => {
     return userTag.name === tagName(user.preferences.language);
@@ -40,10 +36,10 @@ function findTag (user, tagName) {
 function _populateDefaultTasks (user, taskTypes) {
   let defaultsData;
   if (user.registeredThrough === 'habitica-android' || user.registeredThrough === 'habitica-ios') {
-    defaultsData = shared.content.userDefaultsMobile;
+    defaultsData = common.content.userDefaultsMobile;
     user.flags.welcomed = true;
   } else {
-    defaultsData = shared.content.userDefaults;
+    defaultsData = common.content.userDefaults;
   }
   let tagsI = taskTypes.indexOf('tag');
 
@@ -52,7 +48,7 @@ function _populateDefaultTasks (user, taskTypes) {
       let newTag = _.cloneDeep(tag);
 
       // tasks automatically get _id=helpers.uuid() from TaskSchema id.default, but tags are Schema.Types.Mixed - so we need to manually invoke here
-      newTag.id = shared.uuid();
+      newTag.id = common.uuid();
       // Render tag's name in user's language
       newTag.name = newTag.name(user.preferences.language);
       return newTag;
@@ -229,21 +225,21 @@ schema.pre('save', true, function preSaveUser (next, done) {
   // do not calculate achievements if items or achievements are not selected
   if (this.isSelected('items') && this.isSelected('achievements')) {
     // Determines if Beast Master should be awarded
-    let beastMasterProgress = shared.count.beastMasterProgress(this.items.pets);
+    let beastMasterProgress = common.count.beastMasterProgress(this.items.pets);
 
     if (beastMasterProgress >= 90 || this.achievements.beastMasterCount > 0) {
       this.achievements.beastMaster = true;
     }
 
     // Determines if Mount Master should be awarded
-    let mountMasterProgress = shared.count.mountMasterProgress(this.items.mounts);
+    let mountMasterProgress = common.count.mountMasterProgress(this.items.mounts);
 
     if (mountMasterProgress >= 90 || this.achievements.mountMasterCount > 0) {
       this.achievements.mountMaster = true;
     }
 
     // Determines if Triad Bingo should be awarded
-    let dropPetCount = shared.count.dropPetsCurrentlyOwned(this.items.pets);
+    let dropPetCount = common.count.dropPetsCurrentlyOwned(this.items.pets);
     let qualifiesForTriad = dropPetCount >= 90 && mountMasterProgress >= 90;
 
     if (qualifiesForTriad || this.achievements.triadBingoCount > 0) {