From fe1b20359d1111286df40392fa0d00df6e65f108 Mon Sep 17 00:00:00 2001
From: Dom Smith <dom@teneightfive.com>
Date: Tue, 24 Nov 2020 12:10:40 +0000
Subject: [PATCH] fix: Apply transformer to duplicate resources correctly

In some instances, if the same resource is return multiple times within
a collection, for example a person in a list of moves, the transformer
was only being called once because devour would cache that resource within
its deserializer.

This meant that if the same person was being moved on the same day they
wouldn't have the fullname property that parts of the app expect.

This change clears the cache when an item is transformed and sets the
new values so that any future occurances return the correct structure.

This is slightly less efficient that being able to udpate a cached
resource but devour doesn't currently support a method to do this.
---
 .../transformers/transform-resource.js        | 24 +++++++++---
 .../transformers/transform-resource.test.js   | 38 ++++++++++++++++++-
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/common/lib/api-client/transformers/transform-resource.js b/common/lib/api-client/transformers/transform-resource.js
index a43778e1b..0cd4b9025 100644
--- a/common/lib/api-client/transformers/transform-resource.js
+++ b/common/lib/api-client/transformers/transform-resource.js
@@ -2,17 +2,31 @@ const { cloneDeep } = require('lodash')
 
 module.exports = function transformResource(transformer) {
   return function deserializer(item, included) {
+    if (!transformer) {
+      return this.deserialize.resource.call(this, item, included)
+    }
+
     const _this = cloneDeep(this)
     const modelName = this.pluralize.singular(item.type)
 
+    // Ensure we remove this deserializer to avoid infinite loop
     delete _this.models[modelName].options.deserializer
 
-    const deserialized = _this.deserialize.resource.call(_this, item, included)
+    // call devour deserializer
+    const deserializedData = _this.deserialize.resource.call(
+      _this,
+      item,
+      included
+    )
 
-    if (!transformer) {
-      return deserialized
-    }
+    const transformedData = transformer(deserializedData)
+
+    // we need to clear the cache to avoid the original resource being loaded
+    // the cache is a collection and doesn't allow an item to be updated
+    this.deserialize.cache.clear()
+    // set the new transformed data to the cache for future deserializations
+    this.deserialize.cache.set(item.type, item.id, transformedData)
 
-    return transformer(deserialized)
+    return transformedData
   }
 }
diff --git a/common/lib/api-client/transformers/transform-resource.test.js b/common/lib/api-client/transformers/transform-resource.test.js
index 42fee6c8a..e6e57d621 100644
--- a/common/lib/api-client/transformers/transform-resource.test.js
+++ b/common/lib/api-client/transformers/transform-resource.test.js
@@ -3,8 +3,8 @@ const { cloneDeep } = require('lodash')
 const transformResource = require('./transform-resource')
 
 describe('API Client', function () {
-  describe('Models', function () {
-    describe('#transform', function () {
+  describe('Transformers', function () {
+    describe('#transformResource', function () {
       let output, _this, item, included
 
       beforeEach(function () {
@@ -18,11 +18,16 @@ describe('API Client', function () {
             singular: sinon.stub().returnsArg(0),
           },
           deserialize: {
+            cache: {
+              clear: sinon.stub(),
+              set: sinon.stub().returnsArg(1),
+            },
             resource: sinon.stub().returnsArg(0),
           },
         }
         included = []
         item = {
+          id: '12345',
           type: 'book',
           foo: 'bar',
         }
@@ -35,10 +40,23 @@ describe('API Client', function () {
 
         it('should not transform output', function () {
           expect(output).to.deep.equal({
+            id: '12345',
             type: 'book',
             foo: 'bar',
           })
         })
+
+        it('should call original deserializer', function () {
+          expect(_this.deserialize.resource).to.be.calledOnceWithExactly(
+            item,
+            included
+          )
+        })
+
+        it('should not adjust cache', function () {
+          expect(_this.deserialize.cache.clear).not.to.be.called
+          expect(_this.deserialize.cache.set).not.to.be.called
+        })
       })
 
       context('with transformer', function () {
@@ -61,6 +79,7 @@ describe('API Client', function () {
 
         it('should return transformed resource', function () {
           expect(output).to.deep.equal({
+            id: '12345',
             type: 'book',
             foo: 'bar',
             _fizz: 'buzz',
@@ -69,6 +88,7 @@ describe('API Client', function () {
 
         it('should not mutate original item', function () {
           expect(item).to.deep.equal({
+            id: '12345',
             type: 'book',
             foo: 'bar',
           })
@@ -81,6 +101,20 @@ describe('API Client', function () {
           )
         })
 
+        it('should adjust devour cache', function () {
+          expect(_this.deserialize.cache.clear).to.have.been.calledWithExactly()
+          expect(_this.deserialize.cache.set).to.have.been.calledWithExactly(
+            'book',
+            '12345',
+            {
+              id: '12345',
+              type: 'book',
+              foo: 'bar',
+              _fizz: 'buzz',
+            }
+          )
+        })
+
         it.skip('should delete model deserializer', function () {
           const _that = cloneDeep(_this)
           delete _that.models.book.options.deserializer