add tests, move CustomError to common, fix casting

common/locales/en/api-v3.json

@@ -73,7 +73,7 @@
   "guildQuestsNotSupported": "Guilds cannot be invited on quests.",
   "questNotFound": "Quest \"<%= key %>\" not found.",
   "questNotOwned": "You don't own that quest scroll.",
-  "questLevelTooHigh": "You must be Level <%= level %> to begin this quest.",
+  "questLevelTooHigh": "You must be level <%= level %> to begin this quest.",
   "questAlreadyUnderway": "Your party is already on a quest. Try again when the current quest has ended.",
   "questAlreadyAccepted": "You already accepted the quest invitation.",
   "noActiveQuestToLeave": "No active quest to leave",
@@ -90,5 +90,11 @@
   "noAdminAccess": "You don't have admin access.",
   "pageMustBeNumber": "req.query.page must be a number",
   "missingUnsubscriptionCode": "Missing unsubscription code.",
-  "userNotFound": "User not Found"
+  "userNotFound": "User not found.",
+  "spellNotFound": "Spell \"<%= spellId %>\" not found.",
+  "partyNotFound": "Party not found",
+  "targetIdUUID": "\"targetId\" must be a valid UUID.",
+  "challengeTasksNoCast": "Casting a spell on challenge tasks is not supported.",
+  "spellNotOwned": "You don't own this spell.",
+  "spellLevelTooHigh": "You must be level <%= level %> to use this spell."
 }

common/script/api-v3/customError.js

@@ -0,0 +1,8 @@
+// Base class for custom application errors
+// It extends Error and capture the stack trace
+export default class CustomError extends Error {
+  constructor () {
+    super();
+    Error.captureStackTrace(this, this.constructor);
+  }
+}

common/script/api-v3/errors.js

@@ -0,0 +1,9 @@
+import CustomError from './customError';
+
+export class NotAuthorized extends CustomError {
+  constructor (customMessage) {
+    super();
+    this.name = this.constructor.name;
+    this.message = customMessage || 'Not authorized.';
+  }
+}

common/script/content/spells.js

@@ -1,6 +1,6 @@
 import t from './translation';
 import _ from 'lodash';
-
+import { NotAuthorized } from '../api-v3/errors';
 /*
   ---------------------------------------------------------------
   Spells
@@ -40,14 +40,12 @@ spells.wizard = {
     lvl: 11,
     target: 'task',
     notes: t('spellWizardFireballNotes'),
-    cast (user, target) {
+    cast (user, target, req) {
       let bonus = user._statsComputed.int * user.fns.crit('per');
       bonus *= Math.ceil((target.value < 0 ? 1 : target.value + 1) * 0.075);
       user.stats.exp += diminishingReturns(bonus, 75);
       if (!user.party.quest.progress) user.party.quest.progress = 0;
       user.party.quest.progress.up += Math.ceil(user._statsComputed.int * 9.1);
-      // TODO change, pass req to spell?
-      let req = {language: user.preferences.language};
       user.fns.updateStats(user.stats, req);
     },
   },
@@ -166,12 +164,11 @@ spells.rogue = {
     lvl: 12,
     target: 'task',
     notes: t('spellRogueBackStabNotes'),
-    cast (user, target) {
+    cast (user, target, req) {
       let _crit = user.fns.crit('str', 0.3);
       let bonus = calculateBonus(target.value, user._statsComputed.str, _crit);
       user.stats.exp += diminishingReturns(bonus, 75, 50);
       user.stats.gp += diminishingReturns(bonus, 18, 75);
-      let req = {language: user.preferences.language};
       user.fns.updateStats(user.stats, req);
     },
   },
@@ -262,9 +259,11 @@ spells.special = {
     text: t('spellSpecialSnowballAuraText'),
     mana: 0,
     value: 15,
+    previousPurchase: true,
     target: 'user',
     notes: t('spellSpecialSnowballAuraNotes'),
-    cast (user, target) {
+    cast (user, target, req) {
+      if (!user.items.special.snowball) throw new NotAuthorized(t('spellNotOwned')(req.language));
       target.stats.buffs.snowball = true;
       target.stats.buffs.spookDust = false;
       target.stats.buffs.shinySeed = false;
@@ -290,9 +289,11 @@ spells.special = {
     text: t('spellSpecialSpookDustText'),
     mana: 0,
     value: 15,
+    previousPurchase: true,
     target: 'user',
     notes: t('spellSpecialSpookDustNotes'),
-    cast (user, target) {
+    cast (user, target, req) {
+      if (!user.items.special.spookDust) throw new NotAuthorized(t('spellNotOwned')(req.language));
       target.stats.buffs.snowball = false;
       target.stats.buffs.spookDust = true;
       target.stats.buffs.shinySeed = false;
@@ -318,9 +319,11 @@ spells.special = {
     text: t('spellSpecialShinySeedText'),
     mana: 0,
     value: 15,
+    previousPurchase: true,
     target: 'user',
     notes: t('spellSpecialShinySeedNotes'),
-    cast (user, target) {
+    cast (user, target, req) {
+      if (!user.items.special.shinySeed) throw new NotAuthorized(t('spellNotOwned')(req.language));
       target.stats.buffs.snowball = false;
       target.stats.buffs.spookDust = false;
       target.stats.buffs.shinySeed = true;
@@ -346,9 +349,11 @@ spells.special = {
     text: t('spellSpecialSeafoamText'),
     mana: 0,
     value: 15,
+    previousPurchase: true,
     target: 'user',
     notes: t('spellSpecialSeafoamNotes'),
-    cast (user, target) {
+    cast (user, target, req) {
+      if (!user.items.special.seafoam) throw new NotAuthorized(t('spellNotOwned')(req.language));
       target.stats.buffs.snowball = false;
       target.stats.buffs.spookDust = false;
       target.stats.buffs.shinySeed = false;
@@ -499,8 +504,8 @@ _.each(spells, (spellClass) => {
   _.each(spellClass, (spell, key) => {
     spell.key = key;
     let _cast = spell.cast;
-    spell.cast = function castSpell (user, target) {
-      _cast(user, target);
+    spell.cast = function castSpell (user, target, req) {
+      _cast(user, target, req);
       user.stats.mp -= spell.mana;
     };
   });

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

@@ -0,0 +1,172 @@
+import {
+  generateUser,
+  translate as t,
+  createAndPopulateGroup,
+  generateChallenge,
+  sleep,
+} from '../../../../helpers/api-integration/v3';
+
+import { v4 as generateUUID } from 'uuid';
+
+describe('POST /user/class/cast/:spellId', () => {
+  let user;
+
+  beforeEach(async () => {
+    user = await generateUser();
+  });
+
+  it('returns an error if spell does not exist', async () => {
+    await user.update({'stats.class': 'rogue'});
+    let spellId = 'invalidSpell';
+    await expect(user.post(`/user/class/cast/${spellId}`))
+      .to.eventually.be.rejected.and.eql({
+        code: 404,
+        error: 'NotFound',
+        message: t('spellNotFound', {spellId}),
+      });
+  });
+
+  it('returns an error if spell does not exist in user\'s class', async () => {
+    let spellId = 'pickPocket';
+    await expect(user.post(`/user/class/cast/${spellId}`))
+      .to.eventually.be.rejected.and.eql({
+        code: 404,
+        error: 'NotFound',
+        message: t('spellNotFound', {spellId}),
+      });
+  });
+
+  it('returns an error if spell.mana > user.mana', async () => {
+    await user.update({'stats.class': 'rogue'});
+    await expect(user.post(`/user/class/cast/backStab`))
+      .to.eventually.be.rejected.and.eql({
+        code: 401,
+        error: 'NotAuthorized',
+        message: t('notEnoughMana'),
+      });
+  });
+
+  it('returns an error if spell.value > user.gold', async () => {
+    await expect(user.post(`/user/class/cast/birthday`))
+      .to.eventually.be.rejected.and.eql({
+        code: 401,
+        error: 'NotAuthorized',
+        message: t('messageNotEnoughGold'),
+      });
+  });
+
+  it('returns an error if spell.lvl > user.level', async () => {
+    await user.update({'stats.mp': 200, 'stats.class': 'wizard'});
+    await expect(user.post(`/user/class/cast/earth`))
+      .to.eventually.be.rejected.and.eql({
+        code: 401,
+        error: 'NotAuthorized',
+        message: t('spellLevelTooHigh', {level: 13}),
+      });
+  });
+
+  it('returns an error if user doesn\'t own the spell', async () => {
+    await expect(user.post(`/user/class/cast/snowball`))
+      .to.eventually.be.rejected.and.eql({
+        code: 400,
+        error: 'NotAuthorized',
+        message: t('spellNotOwned'),
+      });
+  });
+
+  it('returns an error if targetId is not an UUID', async () => {
+    await expect(user.post('/user/class/cast/spellId?targetId=notAnUUID'))
+      .to.eventually.be.rejected.and.eql({
+        code: 400,
+        error: 'BadRequest',
+        message: t('invalidReqParams'),
+      });
+  });
+
+  it('returns an error if targetId is required but missing', async () => {
+    await user.update({'stats.class': 'rogue', 'stats.lvl': 11});
+    await expect(user.post(`/user/class/cast/pickPocket`))
+      .to.eventually.be.rejected.and.eql({
+        code: 400,
+        error: 'BadRequest',
+        message: t('targetIdUUID'),
+      });
+  });
+
+  it('returns an error if targeted task doesn\'t exist', async () => {
+    await user.update({'stats.class': 'rogue', 'stats.lvl': 11});
+    await expect(user.post(`/user/class/cast/pickPocket?targetId=${generateUUID()}`))
+      .to.eventually.be.rejected.and.eql({
+        code: 404,
+        error: 'NotFound',
+        message: t('taskNotFound'),
+      });
+  });
+
+  it('returns an error if a challenge task was targeted', async () => {
+    let {group, groupLeader} = await createAndPopulateGroup();
+    let challenge = await generateChallenge(groupLeader, group);
+    await groupLeader.post(`/tasks/challenge/${challenge._id}`, [
+      {type: 'habit', text: 'task text'},
+    ]);
+    await groupLeader.update({'stats.class': 'rogue', 'stats.lvl': 11});
+    await sleep(0.5);
+    await groupLeader.sync();
+    await expect(groupLeader.post(`/user/class/cast/pickPocket?targetId=${groupLeader.tasksOrder.habits[0]}`))
+      .to.eventually.be.rejected.and.eql({
+        code: 400,
+        error: 'BadRequest',
+        message: t('challengeTasksNoCast'),
+      });
+  });
+
+  it('returns an error if targeted party member doesn\'t exist', async () => {
+    let {groupLeader} = await createAndPopulateGroup({
+      groupDetails: { type: 'party', privacy: 'private' },
+      members: 1,
+    });
+    await groupLeader.update({'items.special.snowball': 3});
+
+    let target = generateUUID();
+    await expect(groupLeader.post(`/user/class/cast/snowball?targetId=${target}`))
+      .to.eventually.be.rejected.and.eql({
+        code: 404,
+        error: 'NotFound',
+        message: t('userWithIDNotFound', {userId: target}),
+      });
+  });
+
+  it('returns an error if party does not exists', async () => {
+    await user.update({'items.special.snowball': 3});
+
+    await expect(user.post(`/user/class/cast/snowball?targetId=${generateUUID()}`))
+      .to.eventually.be.rejected.and.eql({
+        code: 404,
+        error: 'NotFound',
+        message: t('partyNotFound'),
+      });
+  });
+
+  it('send message in party chat if party && !spell.silent', async () => {
+    let { group, groupLeader } = await createAndPopulateGroup({
+      groupDetails: { type: 'party', privacy: 'private' },
+      members: 1,
+    });
+    await groupLeader.update({'stats.mp': 200, 'stats.class': 'wizard', 'stats.lvl': 13});
+    await groupLeader.post(`/user/class/cast/earth`);
+    await sleep(1);
+    await group.sync();
+    expect(group.chat[0]).to.exists;
+    expect(group.chat[0].uuid).to.equal('system');
+  });
+
+  // TODO find a way to have sinon working in integration tests
+  // it doesn't work when tests are running separately from server
+  it('passes correct target to spell when targetType === \'task\'');
+  it('passes correct target to spell when targetType === \'tasks\'');
+  it('passes correct target to spell when targetType === \'self\'');
+  it('passes correct target to spell when targetType === \'party\'');
+  it('passes correct target to spell when targetType === \'user\'');
+  it('passes correct target to spell when targetType === \'party\' and user is not in a party');
+  it('passes correct target to spell when targetType === \'user\' and user is not in a party');
+});

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

@@ -9,6 +9,7 @@ import responseMiddleware from '../../../../../website/src/middlewares/api-v3/re
 import getUserLanguage from '../../../../../website/src/middlewares/api-v3/getUserLanguage';
 
 import { BadRequest } from '../../../../../website/src/libs/api-v3/errors';
+import { NotAuthorized as NotAuthorizedShared } from '../../../../../common/script/api-v3/errors';
 import logger from '../../../../../website/src/libs/api-v3/logger';
 
 describe('errorHandler', () => {
@@ -86,6 +87,21 @@ describe('errorHandler', () => {
     });
   });
 
+  it('handle CustomError(s) from shared code', () => {
+    let error = new NotAuthorizedShared();
+
+    errorHandler(error, req, res, next);
+
+    expect(res.status).to.be.calledOnce;
+    expect(res.json).to.be.calledOnce;
+
+    expect(res.status).to.be.calledWith(400);
+    expect(res.json).to.be.calledWith({
+      error: 'NotAuthorized',
+      message: 'Not authorized.',
+    });
+  });
+
   it('handle http-errors errors', () => {
     let error = new Error('custom message');
     error.statusCode = 422;

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

@@ -4,6 +4,7 @@ import common from '../../../../common';
 import {
   NotFound,
   BadRequest,
+  NotAuthorized,
 } from '../../libs/api-v3/errors';
 import * as Tasks from '../../models/task';
 import { model as Group } from '../../models/group';
@@ -43,7 +44,7 @@ api.getUser = {
 const partyMembersFields = 'profile.name stats achievements items.special';
 
 /**
- * @api {post} /user/class/cast/:spell Cast a spell on a target.
+ * @api {post} /user/class/cast/:spellId Cast a spell on a target.
  * @apiVersion 3.0.0
  * @apiName UserCast
  * @apiGroup User
@@ -56,7 +57,7 @@ const partyMembersFields = 'profile.name stats achievements items.special';
 api.castSpell = {
   method: 'POST',
   middlewares: [authWithHeaders(), cron],
-  url: '/user/class/cast/:spell',
+  url: '/user/class/cast/:spellId',
   async handler (req, res) {
     let user = res.locals.user;
     let spellId = req.params.spellId;
@@ -71,14 +72,16 @@ api.castSpell = {
     let klass = common.content.spells.special[spellId] ? 'special' : user.stats.class;
     let spell = common.content.spells[klass][spellId];
 
-    if (!spell) throw new NotFound(res.t('spellNotFound', {spell: spellId}));
-    if (spell.mana > user.stats.mp) throw new BadRequest(res.t('notEnoughMana'));
+    if (!spell) throw new NotFound(res.t('spellNotFound', {spellId}));
+    if (spell.mana > user.stats.mp) throw new NotAuthorized(res.t('notEnoughMana'));
+    if (spell.value > user.stats.gp && !spell.previousPurchase) throw new NotAuthorized(res.t('messageNotEnoughGold'));
+    if (spell.lvl > user.stats.lvl) throw new NotAuthorized(res.t('spellLevelTooHigh', {level: spell.lvl}));
+
     let targetType = spell.target;
 
     if (targetType === 'task') {
       if (!targetId) throw new BadRequest(res.t('targetIdUUID'));
 
-      // TODO what about challenge tasks? should casting be disabled on them?
       let task = await Tasks.Task.findOne({
         _id: targetId,
         userId: user._id,
@@ -86,11 +89,11 @@ api.castSpell = {
       if (!task) throw new NotFound(res.t('taskNotFound'));
       if (task.challenge.id) throw new BadRequest(res.t('challengeTasksNoCast'));
 
-      spell.cast(user, task);
+      spell.cast(user, task, req);
       await task.save();
       res.respond(200, task);
     } else if (targetType === 'self') {
-      spell.cast(user);
+      spell.cast(user, null, req);
       await user.save();
       res.respond(200, user);
     } else if (targetType === 'tasks') { // new target type when all the user's tasks are necessary
@@ -103,7 +106,7 @@ api.castSpell = {
         ],
       }).exec();
 
-      spell.cast(user, tasks);
+      spell.cast(user, tasks, req);
 
       let toSave = tasks.filter(t => t.isModified());
       let isUserModified = user.isModified();
@@ -116,8 +119,7 @@ api.castSpell = {
       if (isUserModified) res.user = user;
       res.respond(200, response);
     } else if (targetType === 'party' || targetType === 'user') {
-      let party = await Group.getGroup({_id: 'party', user});
-
+      let party = await Group.getGroup({groupId: 'party', user});
       // arrays of users when targetType is 'party' otherwise single users
       let partyMembers;
 
@@ -128,18 +130,19 @@ api.castSpell = {
           partyMembers = await User.find({'party._id': party._id}).select(partyMembersFields).exec();
         }
 
-        spell.cast(user, partyMembers);
+        spell.cast(user, partyMembers, req);
         await Q.all(partyMembers.map(m => m.save()));
       } else {
         if (!party && (!targetId || user._id === targetId)) {
           partyMembers = user;
         } else {
           if (!targetId) throw new BadRequest(res.t('targetIdUUID'));
+          if (!party) throw new NotFound(res.t('partyNotFound'));
           partyMembers = await User.findOne({_id: targetId, 'party._id': party._id}).select(partyMembersFields).exec();
         }
 
         if (!partyMembers) throw new NotFound(res.t('userWithIDNotFound', {userId: targetId}));
-        spell.cast(user, partyMembers);
+        spell.cast(user, partyMembers, req);
         await partyMembers.save();
       }
       res.respond(200, partyMembers);

website/src/libs/api-v3/errors.js

@@ -1,11 +1,4 @@
-// Base class for custom application errors
-// It extends Error and capture the stack trace
-export class CustomError extends Error {
-  constructor () {
-    super();
-    Error.captureStackTrace(this, this.constructor);
-  }
-}
+import CustomError from '../../../../common/script/api-v3/customError';
 
 /**
  * @apiDefine NotAuthorized

website/src/middlewares/api-v3/errorHandler.js

@@ -1,8 +1,8 @@
 // The error handler middleware that handles all errors
 // and respond to the client
 import logger from '../../libs/api-v3/logger';
+import CustomError from '../../../../common/script/api-v3/customError';
 import {
-  CustomError,
   BadRequest,
   InternalServerError,
 } from '../../libs/api-v3/errors';
@@ -24,11 +24,17 @@ export default function errorHandler (err, req, res, next) { // eslint-disable-l
   // If we can't identify it, respond with a generic 500 error
   let responseErr = err instanceof CustomError ? err : null;
 
+  // TODO don't return always 400 for errors in common code
+  // If CustomError but without httpCode then they come from shared code, treat as 400s
+  if (err instanceof CustomError && !err.httpCode) {
+    err.httpCode = 400;
+  }
+
   // Handle errors created with 'http-errors' or similar that have a status/statusCode property
   if (err.statusCode && typeof err.statusCode === 'number') {
     responseErr = new CustomError();
     responseErr.httpCode = err.statusCode;
-    responseErr.error = err.name;
+    responseErr.name = err.name;
     responseErr.message = err.message;
   }
 

website/src/models/group.js

@@ -125,20 +125,20 @@ schema.statics.getGroup = async function getGroup (options = {}) {
   let {user, groupId, fields, optionalMembership = false, populateLeader = false, requireMembership = false} = options;
   let query;
 
-  let isParty = groupId === 'party' || user.party._id === groupId;
-  let isGuild = user.guilds.indexOf(groupId) !== -1;
+  let isUserParty = groupId === 'party' || user.party._id === groupId;
+  let isUserGuild = user.guilds.indexOf(groupId) !== -1;
 
   // When requireMembership is true check that user is member even in public guild
-  if (requireMembership && !isParty && !isGuild) {
+  if (requireMembership && !isUserParty && !isUserGuild) {
     return null;
   }
 
   // When optionalMembership is true it's not required for the user to be a member of the group
-  if (isParty) {
+  if (isUserParty) {
     query = {type: 'party', _id: user.party._id};
   } else if (optionalMembership === true) {
     query = {_id: groupId};
-  } else if (isGuild) {
+  } else if (isUserGuild) {
     query = {type: 'guild', _id: groupId};
   } else {
     query = {type: 'guild', privacy: 'public', _id: groupId};

website/src/models/user.js

@@ -453,7 +453,7 @@ export let schema = new Schema({
     lvl: {type: Number, default: 1},
 
     // Class System
-    class: {type: String, enum: ['warrior', 'rogue', 'wizard', 'healer'], default: 'warrior'},
+    class: {type: String, enum: ['warrior', 'rogue', 'wizard', 'healer'], default: 'warrior', required: true},
     points: {type: Number, default: 0},
     str: {type: Number, default: 0},
     con: {type: Number, default: 0},