Add delete expenses route

server/constants/activities.js

@@ -6,6 +6,9 @@ module.exports = {
   ACTIVITY_ALL: 'all',
   GROUP_CREATED: 'group.created',
   GROUP_EXPENSE_CREATED: 'group.expense.created',
+  GROUP_EXPENSE_DELETED: 'group.expense.deleted',
+  GROUP_EXPENSE_REJECTED: 'group.expense.rejected',
+  GROUP_EXPENSE_APPROVED: 'group.expense.approved',
   GROUP_TRANSACTION_CREATED: 'group.transaction.created',
   GROUP_TRANSACTION_PAID: 'group.transaction.paid',
   GROUP_USER_ADDED: 'group.user.added',

server/controllers/expenses.js

@@ -3,8 +3,9 @@
  */
 
 const Promise = require('bluebird');
-const constants = require('../constants');
+const activities = require('../constants/activities');
 const includes = require('lodash/collection/includes');
+const status = require('../constants/expense_status');
 
 /**
  * Controller.
@@ -19,7 +20,7 @@ module.exports = (app) => {
   const payExpense = require('../lib/payExpense')(app);
 
   /**
-   * Create an expense and add it to a group.
+   * Create an expense.
    */
 
   const create = (req, res, next) => {
@@ -30,49 +31,47 @@ module.exports = (app) => {
       GroupId: group.id
     });
     models.Expense.create(attributes)
-      .tap(expense => createNewExpenseActivity(expense.id))
+      .then(expense => models.Expense.findById(expense.id, { include: [ models.Group, models.User ]}))
+      .tap(expense => createActivity(expense, activities.GROUP_EXPENSE_CREATED))
       .tap(expense => res.send(expense))
       .catch(next);
+  };
 
-    function createNewExpenseActivity(id) {
-      return models.Expense.findOne({
-        where: { id },
-        include: [
-          { model: models.Group },
-          { model: models.User }
-        ]
-      })
-      .then(expense => models.Activity.create({
-        type: constants.activities.GROUP_EXPENSE_CREATED,
-        UserId: expense.User.id,
-        GroupId: expense.Group.id,
-        data: {
-          group: expense.Group.info,
-          user: expense.User.info,
-          expense: expense.info
-        }
-      }));
-    }
+  /**
+   * Delete an expense.
+   */
+
+  const deleteExpense = (req, res, next) => {
+    const expense = req.expense;
+
+    assertExpenseStatus(expense, status.PENDING)
+      .then(() => expense.destroy())
+      .tap(expense => createActivity(expense, activities.GROUP_EXPENSE_DELETED))
+      .tap(() => res.send({success: true}))
+      .catch(next);
   };
 
   /**
-   * Set the approval status of an expense
+   * Approve or reject an expense.
    */
 
   const setApprovalStatus = (req, res, next) => {
+    const expense = req.expense;
     var preapprovalDetails;
 
-    if (req.required.approved === false) {
-      return req.expense.setRejected()
-        .then(() => res.send({success: true}))
-        .catch(next);
-    }
-
-    fetchPaymentMethod(req.remoteUser.id)
-      .then(paymentMethod => getPreapprovalDetails(paymentMethod))
-      .tap(d => preapprovalDetails = d)
-      .then(checkIfEnoughFunds(req.expense.amount))
-      .then(() => req.expense.setApproved())
+    assertExpenseStatus(expense, status.PENDING)
+      .then(() => {
+        if (req.required.approved === false) {
+          return req.expense.setRejected()
+            .tap(expense => createActivity(expense, activities.GROUP_EXPENSE_REJECTED))
+        }
+        return fetchPaymentMethod(req.remoteUser.id)
+          .then(paymentMethod => getPreapprovalDetails(paymentMethod))
+          .tap(d => preapprovalDetails = d)
+          .then(checkIfEnoughFunds(expense.amount))
+          .then(() => expense.setApproved())
+          .tap(expense => createActivity(expense, activities.GROUP_EXPENSE_APPROVED))
+      })
       .then(() => res.send({success: true}))
       .catch(err => next(formatError(err, preapprovalDetails)));
 
@@ -118,7 +117,7 @@ module.exports = (app) => {
     const isManual = !includes(models.PaymentMethod.payoutMethods, payoutMethod);
     var paymentMethod, email, paymentResponse;
 
-    checkExpenseIsApproved()
+    assertExpenseStatus(expense, status.APPROVED)
       .then(() => isManual ? null : getPaymentMethod())
       .tap(m => paymentMethod = m)
       .then(getBeneficiaryEmail)
@@ -130,13 +129,6 @@ module.exports = (app) => {
       .tap(() => res.json(expense))
       .catch(err => next(formatError(err, paymentResponse)));
 
-    function checkExpenseIsApproved() {
-      if (!expense.isApproved) {
-        return Promise.reject(new errors.BadRequest(`Expense ${expense.id} has not been approved.`));
-      }
-      return Promise.resolve();
-    }
-
     function getPaymentMethod() {
       // Use first paymentMethod found
       return models.PaymentMethod.findOne({
@@ -172,10 +164,31 @@ module.exports = (app) => {
 
   return {
     create,
+    deleteExpense,
     setApprovalStatus,
     pay
   };
 
+  function assertExpenseStatus(expense, status) {
+    if (expense.status !== status) {
+      return Promise.reject(new errors.BadRequest(`Expense ${expense.id} status should be ${status}.`));
+    }
+    return Promise.resolve();
+  }
+
+  function createActivity(expense, type) {
+    return models.Activity.create({
+      type,
+      UserId: expense.User.id,
+      GroupId: expense.Group.id,
+      data: {
+        group: expense.Group.info,
+        user: expense.User.info,
+        expense: expense.info
+      }
+    });
+  }
+
   function formatError(err, paypalResponse) {
     if (paypalResponse) {
       console.error('PayPal error', JSON.stringify(paypalResponse));

server/middleware/params.js

@@ -103,7 +103,13 @@ module.exports = (app) => {
      */
     expenseid: (req, res, next, expenseid) => {
       parseId(expenseid)
-        .then(id => Expense.findById(id))
+        .then(id => Expense.findOne({
+          where: { id },
+          include: [
+            { model: models.Group },
+            { model: models.User }
+          ]
+        }))
         .then((expense) => {
           if (!expense) {
             return next(new errors.NotFound(`Expense '${expenseid}' not found`));

server/models/Expense.js

@@ -100,23 +100,6 @@ module.exports = function (Sequelize, DataTypes) {
     paranoid: true,
 
     getterMethods: {
-
-      isPending() {
-        return this.status === status.PENDING;
-      },
-
-      isApproved() {
-        return this.status === status.APPROVED;
-      },
-
-      isRejected() {
-        return this.status === status.REJECTED;
-      },
-
-      isPaid() {
-        return this.status === status.PAID;
-      },
-
       info() {
         return {
           id: this.id,

server/routes.js

@@ -154,6 +154,7 @@ module.exports = (app) => {
 
   app.get('/groups/:groupid/transactions/:transactionid', aZ.authorizeAccessToGroup({authIfPublic: true}), aZ.authorizeGroupAccessToTransaction({authIfPublic: true}), groups.getTransaction); // Get a transaction.
   app.put('/groups/:groupid/transactions/:transactionid', aZ.authorizeAccessToGroup(), aZ.authorizeGroupAccessToTransaction(), required('transaction'), groups.updateTransaction); // Update a transaction.
+  // TODO remove #postmigration, replaced by DEL /groups/:groupid/expenses/:expenseid
   app.delete('/groups/:groupid/transactions/:transactionid', aZ.authorizeAccessToGroup({userRoles: [HOST], bypassUserRolesCheckIfAuthenticatedAsAppAndNotUser: true}), aZ.authorizeGroupAccessToTransaction(), groups.deleteTransaction); // Delete a transaction.
   // TODO remove #postmigration, replaced by POST /groups/:groupid/expenses/:expenseid/approve
   app.post('/groups/:groupid/transactions/:transactionid/approve', aZ.authorizeAccessToGroup(), aZ.authorizeGroupAccessToTransaction(), required('approved'), transactions.setApprovedState); // Approve a transaction.
@@ -168,8 +169,10 @@ module.exports = (app) => {
   // xdamman: having two times the same route is a mess (hard to read and error prone if we forget to return)
   // This is caused by mw.authorizeIfGroupPublic that is doing a next('route')
   // TODO refactor with single route using authentication.js and authorization.js middleware
-  app.post('/groups/:groupid/expenses', commonLegacySecurityMw, mw.authorizeIfGroupPublic, mw.authorizeAuthUserOrApp, mw.authorizeGroup, required('expense'), mw.getOrCreateUser, expenses.create); // Create an expense for a group.
-  app.post('/groups/:groupid/expenses', commonLegacySecurityMw, required('expense'), mw.getOrCreateUser, expenses.create); // Create an expense for a group.
+  app.post('/groups/:groupid/expenses', commonLegacySecurityMw, mw.authorizeIfGroupPublic, mw.authorizeAuthUserOrApp, mw.authorizeGroup, required('expense'), mw.getOrCreateUser, expenses.create); // Create an expense.
+  app.post('/groups/:groupid/expenses', commonLegacySecurityMw, required('expense'), mw.getOrCreateUser, expenses.create); // Create an expense.
+  // TODO is option bypassUserRolesCheckIfAuthenticatedAsAppAndNotUser present in DEL /groups/:id/transactions/:id really needed?
+  app.delete('/groups/:groupid/expenses/:expenseid', aZ.authorizeAccessToGroup({userRoles: [HOST]}), aZ.authorizeGroupAccessTo('expense'), expenses.deleteExpense); // Delete an expense.
   app.post('/groups/:groupid/expenses/:expenseid/approve', aZ.authorizeAccessToGroup(), aZ.authorizeGroupAccessTo('expense'), required('approved'), expenses.setApprovalStatus); // Approve an expense.
   app.post('/groups/:groupid/expenses/:expenseid/pay', aZ.authorizeAccessToGroup({userRoles: [HOST]}), aZ.authorizeGroupAccessTo('expense'), required('payoutMethod'), expenses.pay); // Pay an expense.