fix: disallow editing invoices that are deleted or paid

src/controller/request/validators/invoice-request-spec.ts

@@ -36,7 +36,7 @@ import {
   INVALID_INVOICE_ID,
   INVALID_TRANSACTION_IDS,
   INVALID_TRANSACTION_OWNER,
-  INVOICE_IS_DELETED, NO_TRANSACTION_IDS,
+  INVOICE_IS_DELETED, INVOICE_IS_PAID, NO_TRANSACTION_IDS,
   SAME_INVOICE_STATE, SUBTRANSACTION_ALREADY_INVOICED,
 } from './validation-errors';
 import { InvoiceState } from '../../../entity/invoices/invoice-status';
@@ -80,13 +80,17 @@ async function validTransactionIds<T extends BaseInvoice>(p: T) {
  * Validates that Invoice exists and is not of state DELETED.
  * @param p
  */
-async function existsAndNotDeleted<T extends UpdateInvoiceParams>(p: T) {
+async function existsAndNotPaidOrDeleted<T extends UpdateInvoiceParams>(p: T) {
   const base: Invoice = await Invoice.findOne({ where: { id: p.invoiceId }, relations: ['invoiceStatus'] });
 
   if (!base) return toFail(INVALID_INVOICE_ID());
-  if (base.invoiceStatus[base.invoiceStatus.length - 1].state === InvoiceState.DELETED) {
+  const current = base.invoiceStatus.sort((a, b) => a.createdAt.getTime() - b.createdAt.getTime())[base.invoiceStatus.length - 1].state;
+  if (current === InvoiceState.DELETED) {
     return toFail(INVOICE_IS_DELETED());
   }
+  if (current === InvoiceState.PAID) {
+    return toFail(INVOICE_IS_PAID());
+  }
 
   return toPass(p);
 }
@@ -122,7 +126,7 @@ function baseInvoiceRequestSpec<T extends BaseInvoice>(): Specification<T, Valid
 const updateInvoiceRequestSpec: Specification<UpdateInvoiceParams, ValidationError> = [
   [stringSpec(), 'description', new ValidationError('description:')],
   differentState,
-  existsAndNotDeleted,
+  existsAndNotPaidOrDeleted,
 ];
 
 /**

src/controller/request/validators/validation-errors.ts

@@ -58,6 +58,8 @@ export const INVALID_INVOICE_ID = () => new ValidationError('Invoice with this I
 
 export const INVOICE_IS_DELETED = () => new ValidationError('Invoice is deleted.');
 
+export const INVOICE_IS_PAID = () => new ValidationError('Invoice is paid.');
+
 export const SAME_INVOICE_STATE = () => new ValidationError('Update state is same as current state.');
 
 export const SUBTRANSACTION_ALREADY_INVOICED = (ids: number[]) => new ValidationError(`SubTransactions ${ids}: have already been invoiced.`);

test/seed/ledger/invoice-seeder.ts

@@ -118,17 +118,16 @@ export default class InvoiceSeeder extends WithManager {
     await this.manager.save(Invoice, invoices);
 
     for (let i = 0; i < invoices.length; i += 1) {
-      if (i % 2 === 0) {
-        const current = invoices[i].invoiceStatus[0].changedBy.id;
-        const status = Object.assign(new InvoiceStatus(), {
-          invoice: invoices[i],
-          changedBy: current,
-          state: InvoiceState.SENT,
-          dateChanged: addDays(new Date(2020, 0, 1), 2 - (i * 2)),
-        });
-        invoices[i].invoiceStatus.push(status);
-        await this.manager.save(Invoice, invoices[i]);
-      }
+      if (i % 4 === 0) continue;
+      const current = invoices[i].invoiceStatus[0].changedBy.id;
+      const status = Object.assign(new InvoiceStatus(), {
+        invoice: invoices[i],
+        changedBy: current,
+        state: [InvoiceState.SENT, InvoiceState.PAID, InvoiceState.DELETED][i % 3],
+        dateChanged: addDays(new Date(2020, 0, 1), 2 - (i * 2)),
+      });
+      invoices[i].invoiceStatus.push(status);
+      await this.manager.save(Invoice, invoices[i]);
     }
 
 

test/unit/controller/invoice-controller.ts

@@ -41,7 +41,8 @@ import {
 import Transaction from '../../../src/entity/transactions/transaction';
 import {
   INVALID_TRANSACTION_OWNER,
-  INVALID_USER_ID, NO_TRANSACTION_IDS,
+  INVALID_USER_ID, INVOICE_IS_DELETED,
+  INVOICE_IS_PAID, NO_TRANSACTION_IDS,
   SAME_INVOICE_STATE,
   SUBTRANSACTION_ALREADY_INVOICED,
   ZERO_LENGTH_STRING,
@@ -73,6 +74,7 @@ describe('InvoiceController', async () => {
     validInvoiceRequest: CreateInvoiceRequest,
     token: string,
     invoiceUser: User,
+    invoices: Invoice[],
   };
 
   before(async () => {
@@ -110,13 +112,29 @@ describe('InvoiceController', async () => {
       acceptedToS: TermsOfServiceStatus.NOT_REQUIRED,
     } as User;
 
+    let invoiceUser3 = {
+      firstName: 'User3',
+      type: UserType.INVOICE,
+      active: true,
+      acceptedToS: TermsOfServiceStatus.NOT_REQUIRED,
+    } as User;
+
+    let invoiceUser4 = {
+      firstName: 'User4',
+      type: UserType.INVOICE,
+      active: true,
+      acceptedToS: TermsOfServiceStatus.NOT_REQUIRED,
+    } as User;
+
     await User.save(adminUser);
     await User.save(localUser);
     await User.save(invoiceUser);
     await User.save(invoiceUser2);
+    await User.save(invoiceUser3);
+    await User.save(invoiceUser4);
 
-    const { transactions } = await new TransactionSeeder().seed([adminUser, localUser, invoiceUser, invoiceUser2]);
-    await new InvoiceSeeder().seed([invoiceUser, invoiceUser2], transactions);
+    const { transactions } = await new TransactionSeeder().seed([adminUser, localUser, invoiceUser, invoiceUser2, invoiceUser3, invoiceUser4]);
+    const { invoices } = await new InvoiceSeeder().seed([invoiceUser, invoiceUser2, invoiceUser3, invoiceUser4], transactions);
 
     const app = express();
     const specification = await Swagger.initialize(app);
@@ -190,6 +208,7 @@ describe('InvoiceController', async () => {
       adminToken,
       token,
       invoiceUser,
+      invoices,
     };
   });
 
@@ -279,8 +298,7 @@ describe('InvoiceController', async () => {
       await expectError(req, NO_TRANSACTION_IDS().value);
     });
     it('should verify that description is a valid string', async () => {
-      const transactionIDs = (await Transaction.find({ relations: ['from'] })).filter((i) => i.from.id === ctx.validInvoiceRequest.forId).map((t) => t.id);
-      const req: CreateInvoiceRequest = { ...ctx.validInvoiceRequest, description: '', transactionIDs };
+      const req: CreateInvoiceRequest = { ...ctx.validInvoiceRequest, description: '' };
       await expectError(req, `description: ${ZERO_LENGTH_STRING().value}`);
     });
     it('should disallow double invoicing of a transaction', async () => {
@@ -514,6 +532,38 @@ describe('InvoiceController', async () => {
       expect(res.status).to.eq(400);
       expect(res.body).to.eq(SAME_INVOICE_STATE().value);
     });
+    it('should verify that invoice is not deleted', async () => {
+      const invoice = ctx.invoices.find((i) => InvoiceService.isState(i, InvoiceState.DELETED));
+      expect(invoice).to.not.be.undefined;
+      const req: UpdateInvoiceRequest = {
+        addressee: 'Updated-addressee',
+        description: 'Updated-description',
+      };
+
+      const res = await request(ctx.app)
+        .patch(`/invoices/${invoice.id}`)
+        .set('Authorization', `Bearer ${ctx.adminToken}`)
+        .send(req);
+
+      expect(res.status).to.eq(400);
+      expect(res.body).to.eq(INVOICE_IS_DELETED().value);
+    });
+    it('should verify that invoice is not paid', async () => {
+      const invoice = ctx.invoices.find((i) => InvoiceService.isState(i, InvoiceState.PAID));
+      expect(invoice).to.not.be.undefined;
+      const req: UpdateInvoiceRequest = {
+        addressee: 'Updated-addressee',
+        description: 'Updated-description',
+      };
+
+      const res = await request(ctx.app)
+        .patch(`/invoices/${invoice.id}`)
+        .set('Authorization', `Bearer ${ctx.adminToken}`)
+        .send(req);
+
+      expect(res.status).to.eq(400);
+      expect(res.body).to.eq(INVOICE_IS_PAID().value);
+    });
   });
   describe('DELETE /invoices/{id}', () => {
     it('should return an HTTP 200 and delete the requested invoice if exists and admin', async () => {