From 833035d7be74fafb3850c795a9b75b107767210a Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <github.erisds@gmail.com>
Date: Wed, 9 Mar 2022 14:22:59 +0000
Subject: [PATCH] Improved coverage of api serializer

- Have ensured we have 100% coverage of core/server/api/shared/serializers/handle.js
- This meant I had to swap around two validation clauses as one was unreachable
- I have done this as I want to make some changes in this area of the codebase, and want to ensure we have tests
  and a clear understanding of what this code does before I change it
---
 core/server/api/shared/serializers/handle.js  |  4 +-
 .../api/shared/serializers/handle.test.js     | 91 +++++++++++++++++--
 2 files changed, 85 insertions(+), 10 deletions(-)

diff --git a/core/server/api/shared/serializers/handle.js b/core/server/api/shared/serializers/handle.js
index 21418709a5..77fb7f782e 100644
--- a/core/server/api/shared/serializers/handle.js
+++ b/core/server/api/shared/serializers/handle.js
@@ -21,11 +21,11 @@ module.exports.input = (apiConfig, apiSerializers, frame) => {
     const tasks = [];
     const sharedSerializers = require('./input');
 
-    if (!apiSerializers) {
+    if (!apiConfig) {
         return Promise.reject(new errors.IncorrectUsageError());
     }
 
-    if (!apiConfig) {
+    if (!apiSerializers) {
         return Promise.reject(new errors.IncorrectUsageError());
     }
 
diff --git a/test/unit/api/shared/serializers/handle.test.js b/test/unit/api/shared/serializers/handle.test.js
index 882fdb16ab..1df53cdfd2 100644
--- a/test/unit/api/shared/serializers/handle.test.js
+++ b/test/unit/api/shared/serializers/handle.test.js
@@ -26,7 +26,7 @@ describe('Unit: api/shared/serializers/handle', function () {
                 });
         });
 
-        it('ensure serializers are called with apiConfig and frame', function () {
+        it('ensure default serializers are called with apiConfig and frame', function () {
             const allStub = sinon.stub();
             sinon.stub(shared.serializers.input.all, 'all').get(() => allStub);
 
@@ -51,12 +51,47 @@ describe('Unit: api/shared/serializers/handle', function () {
             return shared.serializers.handle.input(apiConfig, apiSerializers, frame)
                 .then(() => {
                     stubsToCheck.forEach((stub) => {
-                        stub.calledOnce.should.be.true();
-                        should.equal(stub.args[0][0], apiConfig);
-                        should.equal(stub.args[0][1], frame);
+                        sinon.assert.calledOnceWithExactly(stub, apiConfig, frame);
                     });
                 });
         });
+
+        it('ensure serializers are called with apiConfig and frame if new shared serializer is added', function () {
+            const allStub = sinon.stub();
+            const allBrowseStub = sinon.stub();
+
+            shared.serializers.input.all.browse = allBrowseStub;
+
+            sinon.stub(shared.serializers.input.all, 'all').get(() => allStub);
+
+            const apiSerializers = {
+                all: sinon.stub().resolves(),
+                posts: {
+                    all: sinon.stub().resolves(),
+                    browse: sinon.stub().resolves()
+                }
+            };
+
+            const apiConfig = {docName: 'posts', method: 'browse'};
+            const frame = {};
+
+            const stubsToCheck = [
+                allStub,
+                allBrowseStub,
+                apiSerializers.all,
+                apiSerializers.posts.all,
+                apiSerializers.posts.browse
+            ];
+
+            return shared.serializers.handle.input(apiConfig, apiSerializers, frame)
+                .then(() => {
+                    stubsToCheck.forEach((stub) => {
+                        sinon.assert.calledOnceWithExactly(stub, apiConfig, frame);
+                    });
+
+                    sinon.assert.callOrder(allStub, allBrowseStub, apiSerializers.all, apiSerializers.posts.all, apiSerializers.posts.browse);
+                });
+        });
     });
 
     describe('output', function () {
@@ -80,7 +115,7 @@ describe('Unit: api/shared/serializers/handle', function () {
                 });
         });
 
-        it('ensure serializers are called', function () {
+        it('ensure custom api Serializers are called correctly', function () {
             const apiSerializers = {
                 posts: {
                     add: sinon.stub().resolves()
@@ -90,10 +125,50 @@ describe('Unit: api/shared/serializers/handle', function () {
                 }
             };
 
-            return shared.serializers.handle.output([], {docName: 'posts', method: 'add'}, apiSerializers, {})
+            const response = [];
+            const apiConfig = {docName: 'posts', method: 'add'};
+            const frame = {};
+
+            return shared.serializers.handle.output(response, apiConfig, apiSerializers, frame)
                 .then(() => {
-                    apiSerializers.posts.add.calledOnce.should.be.true();
-                    apiSerializers.users.add.called.should.be.false();
+                    sinon.assert.calledOnceWithExactly(apiSerializers.posts.add, response, apiConfig, frame);
+                    sinon.assert.notCalled(apiSerializers.users.add);
+                });
+        });
+
+        it('ensure "all" serializers are called correctly', function () {
+            const apiSerializers = {
+                all: {
+                    after: sinon.stub().resolves(),
+                    before: sinon.stub().resolves()
+
+                },
+                posts: {
+                    add: sinon.stub().resolves(),
+                    all: sinon.stub().resolves()
+                }
+            };
+
+            const response = [];
+            const apiConfig = {docName: 'posts', method: 'add'};
+            const frame = {};
+
+            const stubsToCheck = [
+                apiSerializers.all.before,
+                apiSerializers.posts.add,
+                apiSerializers.posts.all
+            ];
+
+            return shared.serializers.handle.output(response, apiConfig, apiSerializers, frame)
+                .then(() => {
+                    stubsToCheck.forEach((stub) => {
+                        sinon.assert.calledOnceWithExactly(stub, response, apiConfig, frame);
+                    });
+
+                    // After has a different call signature... is this a intentional?
+                    sinon.assert.calledOnceWithExactly(apiSerializers.all.after, apiConfig, frame);
+
+                    sinon.assert.callOrder(apiSerializers.all.before, apiSerializers.posts.all, apiSerializers.posts.add, apiSerializers.all.after);
                 });
         });
     });