From 251c9c0857535ee20bbcb2fe4c79582cd4b04789 Mon Sep 17 00:00:00 2001
From: Loic Huder <loic.huder@esrf.fr>
Date: Mon, 24 Aug 2020 16:29:21 +0200
Subject: [PATCH 1/2] Fix bug in deleteItem and getParcelsByStatus

---
 app/controllers/item.controller.js   | 2 +-
 app/controllers/parcel.controller.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/item.controller.js b/app/controllers/item.controller.js
index bf49be11..8d2ffb7d 100644
--- a/app/controllers/item.controller.js
+++ b/app/controllers/item.controller.js
@@ -114,7 +114,7 @@ function deleteItem(req, res) {
         global.gLogger.warn("Delete item from parcel returned not ok");
       }
       helper
-        .findParcel({ investigationId: req.params.investigationId, _id: req.params.parcelId })
+        .findParcels({ investigationId: req.params.investigationId, _id: req.params.parcelId })
         .then((parcel) => {
           /**Remove item from Items collection */
           Item.deleteOne({ _id: req.body._id })
diff --git a/app/controllers/parcel.controller.js b/app/controllers/parcel.controller.js
index a008b178..05a479b0 100644
--- a/app/controllers/parcel.controller.js
+++ b/app/controllers/parcel.controller.js
@@ -177,7 +177,7 @@ function getParcelsByStatus(req, res) {
   //TODO: this would be much more efficient if we can use the query instead of filtering all results
   // `status` is a virtual field and cannot be queried directly (https://mongoosejs.com/docs/tutorials/virtuals.html#limitations)
   helper
-    .findParcel({})
+    .findParcels({})
     .then((parcels) => {
       const filteredByStatus = parcels.filter((parcel) => parcel.status === req.params.status.toUpperCase());
       res.send(filteredByStatus);
-- 
GitLab


From cb5ec6436835591d99f63755dafdd07afb38e9ed Mon Sep 17 00:00:00 2001
From: Loic Huder <loic.huder@esrf.fr>
Date: Mon, 24 Aug 2020 16:29:42 +0200
Subject: [PATCH 2/2] Updated address and parcel tests

---
 test/global.js                     |  5 +-
 test/routes/test.address.routes.js | 49 ++++++++------------
 test/routes/test.parcel.routes.js  | 74 ++++++++++++------------------
 3 files changed, 50 insertions(+), 78 deletions(-)

diff --git a/test/global.js b/test/global.js
index 336935b1..64f3c68b 100644
--- a/test/global.js
+++ b/test/global.js
@@ -4,7 +4,6 @@ global.gResource = require("./resources/global.resource.js");
 global.gResource.investigations = require("./resources/investigations.resource.js");
 global.chai = require("chai");
 global.expect = global.chai.expect;
-global.should = global.chai.should();
 global.chaiHttp = require("chai-http");
 global.chai.use(global.chaiHttp);
 
@@ -27,7 +26,7 @@ before((done) => {
           //initialize the icat+ server
           global.gServer = require("../server.js");
           global.gServer.onCacheInitialized = function () {
-            global.gRequester = chai.request(global.gServer).keepOpen();
+            global.gRequester = global.chai.request(global.gServer).keepOpen();
             console.log("Server is listening...");
             setTimeout(() => {
               done();
@@ -47,7 +46,7 @@ before((done) => {
   } else {
     global.gServer = require("../server.js");
     global.gServer.onCacheInitialized = function () {
-      global.gRequester = chai.request(global.gServer).keepOpen();
+      global.gRequester = global.chai.request(global.gServer).keepOpen();
       console.log("Server is listening...");
       setTimeout(() => {
         done();
diff --git a/test/routes/test.address.routes.js b/test/routes/test.address.routes.js
index 33c6363a..1393d8d3 100644
--- a/test/routes/test.address.routes.js
+++ b/test/routes/test.address.routes.js
@@ -14,9 +14,9 @@ describe("Tracking Address", () => {
       try {
         const response = await addressHelper.createAddress(element.user, element.address, element.investigationId);
         expect(response.status).to.equal(element.expected.status);
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -29,9 +29,9 @@ describe("Tracking Address", () => {
       try {
         const getAddressByInvestigationIdResponse = await addressHelper.getAddressByInvestigationId(element.user, element.investigationId);
         expect(getAddressByInvestigationIdResponse.status).to.equal(element.expected.status);
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -40,18 +40,16 @@ describe("Tracking Address", () => {
    * Get Address by sessionId
    */
   describe("GET /tracking/{sessionId}/address", () => {
-    it.each(addressResource.getAddressByUsername, "%s", ["description"], (element, next) => {
-      sessionHelper.doGetSession(element.user).then((getSessionResponse) => {
+    it.each(addressResource.getAddressByUsername, "%s", ["description"], async (element, next) => {
+      try {
+        const getSessionResponse = await sessionHelper.doGetSession(element.user);
         expect(getSessionResponse.status).equal(200);
-        global.gRequester
-          .get(`/tracking/${getSessionResponse.body.sessionId}/address`)
-          .set("Content-Type", "application/json")
-          .send()
-          .end((err, response) => {
-            expect(response.status).to.equal(element.expected.status);
-            next();
-          });
-      });
+        const response = await global.gRequester.get(`/tracking/${getSessionResponse.body.sessionId}/address`).set("Content-Type", "application/json").send();
+        expect(response.status).to.equal(element.expected.status);
+        return next();
+      } catch (err) {
+        return next(err);
+      }
     });
   });
 
@@ -72,11 +70,11 @@ describe("Tracking Address", () => {
         const updateAddressResponse = await addressHelper.updateAddress(element.addressUpdator, updatedAddress, element.investigationId);
         expect(updateAddressResponse.status).to.equal(element.expected.status);
         if (updateAddressResponse.status < 300) {
-          testAdress(updateAddressResponse.body, element.expected.address);
+          expect(updateAddressResponse.body).to.include(element.expected.address);
         }
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -99,21 +97,10 @@ describe("Tracking Address", () => {
         if (element.expected.status < 300) {
           expect(deleteAddressResponse.body.status).to.be.equal("REMOVED");
         }
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
 });
-
-/**
- * Test function which tests that an actual tag corresponds to the expected tag.
- * @param {object} actualTag the tag to test
- * @param {object} expectedTag the expected tag
- */
-function testAdress(actualAddress, expectedAddres) {
-  for (const [key, value] of Object.entries(expectedAddres)) {
-    expect(actualAddress[key]).to.equal(value);
-  }
-}
diff --git a/test/routes/test.parcel.routes.js b/test/routes/test.parcel.routes.js
index d32693cd..70b5bae9 100644
--- a/test/routes/test.parcel.routes.js
+++ b/test/routes/test.parcel.routes.js
@@ -15,19 +15,7 @@ describe("Tracking Parcel", () => {
         const createParcelResponse = await parcelHelper.createParcel(element.parcelCreator, element.parcel, element.investigationId, createShipmentResponse.body._id);
         expect(createParcelResponse.status).to.equal(200);
 
-        /**var addItemResponse = await parcelHelper.addItem(
-          element.user,
-          element.investigationId,
-          createParcelResponse.body.parcels[0],
-          element.item
-        );
-        expect(addItemResponse.status).to.equal(element.expected.status);
-        **/
-        for (let i = 0; i < element.statuses.length; i++) {
-          const status = element.statuses[i].status;
-          const user = element.statuses[i].user;
-          const containsDangerousGoods = element.statuses[i].containsDangerousGoods;
-
+        element.statuses.map(async ({ status, user, containsDangerousGoods }) => {
           const setParcelStatusResponse = await parcelHelper.setParcelStatus(user, element.investigationId, createParcelResponse.body.parcels[0], status, {
             containsDangerousGoods,
           });
@@ -35,11 +23,11 @@ describe("Tracking Parcel", () => {
 
           const getParcelByIdResponse = await parcelHelper.getParcelById(element.user, createParcelResponse.body.investigationId, createParcelResponse.body.parcels[0]);
           expect(getParcelByIdResponse.body.status).to.equal(status);
-        }
+        });
 
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -52,9 +40,9 @@ describe("Tracking Parcel", () => {
         element.parcel.shipmentId = createShipmentResponse.body._id;
         const createParcelResponse = await parcelHelper.createParcel(element.parcelCreator, element.parcel, element.investigationId, createShipmentResponse.body._id);
         expect(createParcelResponse.status).to.equal(element.expected.status);
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -71,9 +59,9 @@ describe("Tracking Parcel", () => {
             expect(parcel.status).to.equal(element.status);
           });
         }
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -91,26 +79,24 @@ describe("Tracking Parcel", () => {
         if (element.expected.status < 300) {
           expect(getParcelByShipmentIdResponse.body).to.be.an("array").to.have.lengthOf(1);
         }
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
 
   describe("GET /tracking/{sessionId}/parcel", () => {
-    it.each(resource.getParcelBySessionId, "%s", ["description"], (element, next) => {
-      sessionHelper.doGetSession(element.user).then((getSessionResponse) => {
+    it.each(resource.getParcelBySessionId, "%s", ["description"], async (element, next) => {
+      try {
+        const getSessionResponse = await sessionHelper.doGetSession(element.user);
         expect(getSessionResponse.status).equal(200);
-        global.gRequester
-          .get(`/tracking/${getSessionResponse.body.sessionId}/parcel`)
-          .set("Content-Type", "application/json")
-          .send()
-          .end((err, response) => {
-            expect(response.status).to.equal(element.expected.status);
-            next();
-          });
-      });
+        const response = await global.gRequester.get(`/tracking/${getSessionResponse.body.sessionId}/parcel`).set("Content-Type", "application/json").send();
+        expect(response.status).to.equal(element.expected.status);
+        return next();
+      } catch (e) {
+        return next(e);
+      }
     });
   });
 
@@ -129,9 +115,9 @@ describe("Tracking Parcel", () => {
           expect(getParcelByShipmentIdResponse.body).not.to.be.an("array");
           expect(parcelId).to.be.equal(getParcelByShipmentIdResponse.body._id);
         }
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -153,9 +139,9 @@ describe("Tracking Parcel", () => {
         if (element.expected.status < 300) {
           expect(addItemResponse.body.items).to.be.an("array").to.have.lengthOf(1);
         }
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -163,7 +149,7 @@ describe("Tracking Parcel", () => {
   /**
    * Creates a shipment, then a parcel, then removes the parcel
    */
-  describe("XXDELETE /tracking/:sessionId/investigation/id/:investigationId/parcel/id/:parcelId", () => {
+  describe("DELETE /tracking/:sessionId/investigation/id/:investigationId/parcel/id/:parcelId", () => {
     it.each(resource.deleteParcel, "%s", ["description"], async (element, next) => {
       try {
         const createShipmentResponse = await shipmentHelper.createShipment(element.shipmentCreator, element.shipment, element.investigationId);
@@ -176,9 +162,9 @@ describe("Tracking Parcel", () => {
 
         expect(deleteParcelResponse.status).to.equal(element.expected.status);
 
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -203,9 +189,9 @@ describe("Tracking Parcel", () => {
         const removeItemResponse = await parcelHelper.removeItem(element.user, element.investigationId, createParcelResponse.body.parcels[0], addItemResponse.body.items[0]);
 
         expect(removeItemResponse.status).to.be.equal(element.expected.status);
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
@@ -233,9 +219,9 @@ describe("Tracking Parcel", () => {
         );
 
         expect(transferItemResponse.status).to.be.equal(element.expected.status);
-        next();
+        return next();
       } catch (e) {
-        next(e);
+        return next(e);
       }
     });
   });
-- 
GitLab