Don't bill (subtract gems) multiple times for multiple unlock item set calls (#12116)

* Issue 11050 - Small tuneups to unlock.js

- Use includes i.o. indexOf
- Extract small function for object setting duplication
- Use every instead of custom counter

* Issue 11050 - Properly store purchased items when purchasing them

* Issue 11050 - Couple more tuneups in unlock.js and implemented partial failure scenario

* Issue 11050 - Fix last lint issue

* Issue 11050 - Check path for gear i.o. failing to write it to purchased

* Issue 11050 - Guarantee variation coverage in tests

* Issue 11050 - Use startsWith instead of includes for background check

* Issue 11050 - Don't unlock lost items

package.json

@@ -100,6 +100,7 @@
     "client:build": "cd website/client && npm run build",
     "client:unit": "cd website/client && npm run test:unit",
     "start": "gulp nodemon",
+    "debug": "gulp nodemon --inspect",
     "postinstall": "gulp build && cd website/client && npm install",
     "apidoc": "gulp apidoc"
   },

test/api/v3/integration/user/POST-user_unlock.js

@@ -6,6 +6,7 @@ import {
 describe('POST /user/unlock', () => {
   let user;
   const unlockPath = 'shirt.convict,shirt.cross,shirt.fire,shirt.horizon,shirt.ocean,shirt.purple,shirt.rainbow,shirt.redblue,shirt.thunder,shirt.tropical,shirt.zombie';
+  const unlockGearSetPath = 'items.gear.owned.headAccessory_special_bearEars,items.gear.owned.headAccessory_special_cactusEars,items.gear.owned.headAccessory_special_foxEars,items.gear.owned.headAccessory_special_lionEars,items.gear.owned.headAccessory_special_pandaEars,items.gear.owned.headAccessory_special_pigEars,items.gear.owned.headAccessory_special_tigerEars,items.gear.owned.headAccessory_special_wolfEars';
   const unlockCost = 1.25;
   const usersStartingGems = 5;
 
@@ -34,4 +35,25 @@ describe('POST /user/unlock', () => {
     expect(response.message).to.equal(t('unlocked'));
     expect(user.balance).to.equal(usersStartingGems - unlockCost);
   });
+
+  it('does not reduce a user\'s balance twice', async () => {
+    await user.update({
+      balance: usersStartingGems,
+    });
+    const response = await user.post(`/user/unlock?path=${unlockGearSetPath}`);
+    await user.sync();
+
+    expect(response.message).to.equal(t('unlocked'));
+    expect(user.balance).to.equal(usersStartingGems - unlockCost);
+
+    expect(user.post(`/user/unlock?path=${unlockGearSetPath}`))
+      .to.eventually.be.rejected.and.to.eql({
+        code: 401,
+        error: 'NotAuthorized',
+        message: t('alreadyUnlocked'),
+      });
+    await user.sync();
+
+    expect(user.balance).to.equal(usersStartingGems - unlockCost);
+  });
 });

test/common/ops/unlock.js

@@ -1,12 +1,7 @@
 import unlock from '../../../website/common/script/ops/unlock';
 import i18n from '../../../website/common/script/i18n';
-import {
+import { generateUser } from '../../helpers/common.helper';
-  generateUser,
+import { NotAuthorized, BadRequest } from '../../../website/common/script/libs/errors';
-} from '../../helpers/common.helper';
-import {
-  NotAuthorized,
-  BadRequest,
-} from '../../../website/common/script/libs/errors';
 
 describe('shared.ops.unlock', () => {
   let user;
@@ -31,6 +26,15 @@ describe('shared.ops.unlock', () => {
     }
   });
 
+  it('does not unlock lost gear', done => {
+    user.items.gear.owned.headAccessory_special_bearEars = false;
+
+    unlock(user, { query: { path: 'items.gear.owned.headAccessory_special_bearEars' } });
+
+    expect(user.balance).to.equal(usersStartingGems);
+    done();
+  });
+
   it('returns an error when user balance is too low', done => {
     user.balance = 0;
 
@@ -50,18 +54,30 @@ describe('shared.ops.unlock', () => {
     } catch (err) {
       expect(err).to.be.an.instanceof(NotAuthorized);
       expect(err.message).to.equal(i18n.t('alreadyUnlocked'));
+      expect(user.balance).to.equal(3.75);
       done();
     }
   });
 
-  // disabled until fully implemente
+  it('returns an error when user already owns a full set of gear', done => {
-  xit('returns an error when user already owns items in a full set', done => {
     try {
-      unlock(user, { query: { path: unlockPath } });
+      unlock(user, { query: { path: unlockGearSetPath } });
-      unlock(user, { query: { path: unlockPath } });
+      unlock(user, { query: { path: unlockGearSetPath } });
     } catch (err) {
       expect(err).to.be.an.instanceof(NotAuthorized);
       expect(err.message).to.equal(i18n.t('alreadyUnlocked'));
+      expect(user.balance).to.equal(3.75);
+      done();
+    }
+  });
+
+  xit('returns an error when user already owns items in a full set', done => {
+    try {
+      unlock(user, { query: { path: unlockPath.split(',').splice(2).join(',') } });
+      unlock(user, { query: { path: unlockPath } });
+    } catch (err) {
+      expect(err).to.be.an.instanceof(NotAuthorized);
+      expect(err.message).to.equal(i18n.t('alreadyUnlockedPart'));
       done();
     }
   });
@@ -78,7 +94,7 @@ describe('shared.ops.unlock', () => {
     expect(user.preferences.background).to.equal('giant_florals');
   });
 
-  it('un-equips an item already equipped', () => {
+  it('un-equips a background already equipped', () => {
     expect(user.purchased.background.giant_florals).to.not.exist;
 
     unlock(user, { query: { path: backgroundUnlockPath } }); // unlock
@@ -105,7 +121,7 @@ describe('shared.ops.unlock', () => {
     expect(user.items.gear.owned.headAccessory_special_wolfEars).to.be.true;
   });
 
-  it('unlocks a an item', () => {
+  it('unlocks an item', () => {
     const [, message] = unlock(user, { query: { path: backgroundUnlockPath } });
 
     expect(message).to.equal(i18n.t('unlocked'));

website/client/package-lock.json

@@ -15158,11 +15158,6 @@
         "sha.js": "^2.4.8"
       }
     },
-    "perfect-scrollbar": {
-      "version": "1.5.0",
-      "resolved": "https://registry.npmjs.org/perfect-scrollbar/-/perfect-scrollbar-1.5.0.tgz",
-      "integrity": "sha512-NrNHJn5mUGupSiheBTy6x+6SXCFbLlm8fVZh9moIzw/LgqElN5q4ncR4pbCBCYuCJ8Kcl9mYM0NgDxvW+b4LxA=="
-    },
     "performance-now": {
       "version": "2.1.0",
       "resolved": "https://registry.npmjs.org/performance-now/-/performance-now-2.1.0.tgz",
@@ -15433,17 +15428,6 @@
         }
       }
     },
-    "postcss-import": {
-      "version": "12.0.1",
-      "resolved": "https://registry.npmjs.org/postcss-import/-/postcss-import-12.0.1.tgz",
-      "integrity": "sha512-3Gti33dmCjyKBgimqGxL3vcV8w9+bsHwO5UrBawp796+jdardbcFl4RP5w/76BwNL7aGzpKstIfF9I+kdE8pTw==",
-      "requires": {
-        "postcss": "^7.0.1",
-        "postcss-value-parser": "^3.2.3",
-        "read-cache": "^1.0.0",
-        "resolve": "^1.1.7"
-      }
-    },
     "postcss-load-config": {
       "version": "2.1.0",
       "resolved": "https://registry.npmjs.org/postcss-load-config/-/postcss-load-config-2.1.0.tgz",
@@ -16633,21 +16617,6 @@
         "lodash": "^4.0.1"
       }
     },
-    "read-cache": {
-      "version": "1.0.0",
-      "resolved": "https://registry.npmjs.org/read-cache/-/read-cache-1.0.0.tgz",
-      "integrity": "sha1-5mTvMRYRZsl1HNvo28+GtftY93Q=",
-      "requires": {
-        "pify": "^2.3.0"
-      },
-      "dependencies": {
-        "pify": {
-          "version": "2.3.0",
-          "resolved": "https://registry.npmjs.org/pify/-/pify-2.3.0.tgz",
-          "integrity": "sha1-7RQaasBDqEnqWISY59yosVMw6Qw="
-        }
-      }
-    },
     "read-pkg": {
       "version": "5.2.0",
       "resolved": "https://registry.npmjs.org/read-pkg/-/read-pkg-5.2.0.tgz",
@@ -19722,16 +19691,6 @@
       "resolved": "https://registry.npmjs.org/vue-template-es2015-compiler/-/vue-template-es2015-compiler-1.9.1.tgz",
       "integrity": "sha512-4gDntzrifFnCEvyoO8PqyJDmguXgVPxKiIxrBKjIowvL9l+N66196+72XVYR8BBf1Uv1Fgt3bGevJ+sEmxfZzw=="
     },
-    "vue2-perfect-scrollbar": {
-      "version": "1.4.0",
-      "resolved": "https://registry.npmjs.org/vue2-perfect-scrollbar/-/vue2-perfect-scrollbar-1.4.0.tgz",
-      "integrity": "sha512-kluthjiZOOhAZ/18RTZJr2Y9lpjgkuOuxkH8MMMq1dYrSUdvlEv8V1UPtW7UDVcTAUo048AUE/W4hSPTfluejw==",
-      "requires": {
-        "cssnano": "^4.1.3",
-        "perfect-scrollbar": "^1.4.0",
-        "postcss-import": "^12.0.0"
-      }
-    },
     "vuedraggable": {
       "version": "2.23.2",
       "resolved": "https://registry.npmjs.org/vuedraggable/-/vuedraggable-2.23.2.tgz",

website/client/src/components/messages/messageList.vue

@@ -6,7 +6,7 @@
     <div class="row loadmore">
       <div v-if="canLoadMore && !isLoading">
         <div class="loadmore-divider-holder">
-        <div class="loadmore-divider"></div>
+          <div class="loadmore-divider"></div>
         </div>
         <button
           class="btn btn-secondary"
@@ -14,9 +14,9 @@
         >
           {{ $t('loadEarlierMessages') }}
         </button>
-      <div class="loadmore-divider-holder">
+        <div class="loadmore-divider-holder">
-        <div class="loadmore-divider"></div>
+          <div class="loadmore-divider"></div>
-      </div>
+        </div>
       </div>
       <h2
         v-show="isLoading"

website/client/src/pages/private-messages.vue

@@ -72,8 +72,8 @@
           />
         </div>
         <button
-          class="btn btn-secondary"
           v-if="canLoadMoreConversations"
+          class="btn btn-secondary"
           @click="loadConversations()"
         >
           {{ $t('loadMore') }}

website/common/script/ops/unlock.js

@@ -1,18 +1,71 @@
 import get from 'lodash/get';
-import each from 'lodash/each';
-import pick from 'lodash/pick';
 import setWith from 'lodash/setWith';
 import i18n from '../i18n';
-import splitWhitespace from '../libs/splitWhitespace';
+import { NotAuthorized, BadRequest } from '../libs/errors';
-import {
-  NotAuthorized,
-  BadRequest,
-} from '../libs/errors';
 
 import { removeItemByPath } from './pinnedGearUtils';
 import getItemInfo from '../libs/getItemInfo';
 import content from '../content/index';
 
+const incentiveBackgrounds = ['blue', 'green', 'red', 'purple', 'yellow'];
+
+function determineCost (isBackground, isFullSet) {
+  if (isBackground) {
+    return isFullSet ? 3.75 : 1.75;
+  }
+  return isFullSet ? 1.25 : 0.5;
+}
+
+function isGear (path) {
+  return path.includes('gear.');
+}
+
+function alreadyUnlocked (user, path) {
+  return isGear(path)
+    ? get(user, path) !== undefined
+    : get(user, `purchased.${path}`);
+}
+
+function setAsObject (target, key, value) {
+  // Using Object so path[1] won't create an array but an object {path: {1: value}}
+  setWith(target, key, value, Object);
+}
+
+/**
+ * Splits `items.gear.owned.headAccessory_wolfEars` into `items.gear.owned`
+ * and `headAccessory_wolfEars`
+ */
+function splitPathItem (path) {
+  return path.match(/(.+)\.([^.]+)/).splice(1);
+}
+
+/**
+ * `markModified` does not exist on frontend users
+ */
+function markModified (user, path) {
+  if (user.markModified) user.markModified(path);
+}
+
+function purchaseItem (path, user) {
+  if (isGear(path)) {
+    setAsObject(user, path, true);
+    const itemName = splitPathItem(path)[1];
+    removeItemByPath(user, `gear.flat.${itemName}`);
+    if (path.includes('gear.owned')) markModified(user, 'items.gear.owned');
+  } else {
+    setAsObject(user, `purchased.${path}`, true);
+    markModified(user, 'purchased');
+  }
+}
+
+function buildResponse ({ purchased, preference, items }, ownsAlready, language) {
+  const response = [
+    { purchased, preference, items },
+  ];
+  if (!ownsAlready) response.push(i18n.t('unlocked', language));
+  return response;
+}
+
 // If item is already purchased -> equip it
 // Otherwise unlock it
 export default function unlock (user, req = {}, analytics) {
@@ -22,86 +75,45 @@ export default function unlock (user, req = {}, analytics) {
     throw new BadRequest(i18n.t('pathRequired', req.language));
   }
 
-  const isFullSet = path.indexOf(',') !== -1;
+  const isFullSet = path.includes(',');
-  const isBackground = path.indexOf('background.') !== -1;
+  const isBackground = path.startsWith('background.');
+  const cost = determineCost(isBackground, isFullSet);
 
-  let cost;
+  const setPaths = path.split(',');
-  if (isBackground && isFullSet) {
+  let unlockedAlready;
-    cost = 3.75;
-  } else if (isBackground) {
-    cost = 1.75;
-  } else if (isFullSet) {
-    cost = 1.25;
-  } else {
-    cost = 0.5;
-  }
-
-  let setPaths;
-  let alreadyOwns;
 
   if (isFullSet) {
-    setPaths = path.split(',');
+    const alreadyUnlockedItems = setPaths.filter(p => alreadyUnlocked(user, p)).length;
-    let alreadyOwnedItems = 0;
+    const totalItems = setPaths.length;
-
+    if (alreadyUnlockedItems === totalItems) {
-    each(setPaths, singlePath => {
-      if (get(user, `purchased.${singlePath}`) === true) {
-        alreadyOwnedItems += 1;
-      }
-    });
-
-    if (alreadyOwnedItems === setPaths.length) {
       throw new NotAuthorized(i18n.t('alreadyUnlocked', req.language));
-    // TODO write math formula to check if buying
+    // TODO Different pull request
-    // the full set is cheaper than the items individually
+    // } else if ((totalItems - alreadyOwnedItems) < 3) {
-    // (item cost * number of remaining items) < setCost`
+    //   throw new NotAuthorized(i18n.t('alreadyUnlockedPart', req.language));
-    } /* else if (alreadyOwnedItems > 0) {
+    }
-      throw new NotAuthorized(i18n.t('alreadyUnlockedPart', req.language));
-    } */
   } else {
-    alreadyOwns = get(user, `purchased.${path}`) === true;
+    unlockedAlready = alreadyUnlocked(user, path);
   }
 
-  if (isBackground && !alreadyOwns && (path.indexOf('.blue') !== -1 || path.indexOf('.green') !== -1 || path.indexOf('.red') !== -1 || path.indexOf('.purple') !== -1 || path.indexOf('.yellow') !== -1)) {
+  if (isBackground && !unlockedAlready
+      && incentiveBackgrounds.some(background => path.includes(`.${background}`))) {
     throw new BadRequest(i18n.t('incentiveBackgroundsUnlockedWithCheckins'));
   }
 
-  if ((!user.balance || user.balance < cost) && !alreadyOwns) {
+  if ((!user.balance || user.balance < cost) && !unlockedAlready) {
     throw new NotAuthorized(i18n.t('notEnoughGems', req.language));
   }
 
   if (isFullSet) {
-    each(setPaths, pathPart => {
+    setPaths.forEach(pathPart => purchaseItem(pathPart, user));
-      if (path.indexOf('gear.') !== -1) {
-        // Using Object so path[1] won't create an array but an object {path: {1: value}}
-        setWith(user, pathPart, true, Object);
-        const itemName = pathPart.split('.').pop();
-        removeItemByPath(user, `gear.flat.${itemName}`);
-        if (user.markModified && path.indexOf('gear.owned') !== -1) user.markModified('items.gear.owned');
-      }
-
-      // Using Object so path[1] won't create an array but an object {path: {1: value}}
-      setWith(user, `purchased.${pathPart}`, true, Object);
-    });
   } else {
-    const split = path.split('.');
+    const [key, value] = splitPathItem(path);
-    let value = split.pop();
-    const key = split.join('.');
 
-    if (alreadyOwns) { // eslint-disable-line no-lonely-if
+    if (unlockedAlready) {
-      if (key === 'background' && value === user.preferences.background) {
+      const unsetBackground = isBackground && value === user.preferences.background;
-        value = '';
+      setAsObject(user, `preferences.${key}`, unsetBackground ? '' : value);
-      }
-
-      // Using Object so path[1] won't create an array but an object {path: {1: value}}
-      setWith(user, `preferences.${key}`, value, Object);
     } else {
-      if (path.indexOf('gear.') !== -1) {
+      purchaseItem(path, user);
-        // Using Object so path[1] won't create an array but an object {path: {1: value}}
-        setWith(user, path, true, Object);
-        if (user.markModified && path.indexOf('gear.owned') !== -1) user.markModified('items.gear.owned');
-      }
-      // Using Object so path[1] won't create an array but an object {path: {1: value}}
-      setWith(user, `purchased.${path}`, true, Object);
 
       // @TODO: Test and check test coverage
       if (isBackground) {
@@ -112,11 +124,7 @@ export default function unlock (user, req = {}, analytics) {
     }
   }
 
-  if (!alreadyOwns) {
+  if (!unlockedAlready) {
-    if (path.indexOf('gear.') === -1) {
-      if (user.markModified) user.markModified('purchased');
-    }
-
     user.balance -= cost;
 
     if (analytics) {
@@ -132,11 +140,5 @@ export default function unlock (user, req = {}, analytics) {
     }
   }
 
-  const response = [
+  return buildResponse(user, unlockedAlready, req.language);
-    pick(user, splitWhitespace('purchased preferences items')),
-  ];
-
-  if (!alreadyOwns) response.push(i18n.t('unlocked', req.language));
-
-  return response;
 }