From 511e71f63a2a7c3c5f67f8d95ee1de5fe0edc715 Mon Sep 17 00:00:00 2001
From: Chi Vinh Le <vinh@wikiwi.io>
Date: Mon, 20 Nov 2017 20:19:58 +0100
Subject: [PATCH] Change default settings=null to settings={}

---
 models/asset.js                |  2 +-
 services/assets.js             | 44 +++++++++++++++++++++++++---------
 test/server/services/assets.js |  2 +-
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/models/asset.js b/models/asset.js
index b2e49dcc0..4a6aea7e0 100644
--- a/models/asset.js
+++ b/models/asset.js
@@ -45,7 +45,7 @@ const AssetSchema = new Schema({
   // always after running `rectifySettings` against it.
   settings: {
     type: Schema.Types.Mixed,
-    default: null
+    default: {},
   },
 
   // Tags are added by the self or by administrators.
diff --git a/services/assets.js b/services/assets.js
index 3ab7ec9ff..e2ba0fe04 100644
--- a/services/assets.js
+++ b/services/assets.js
@@ -4,6 +4,7 @@ const SettingsService = require('./settings');
 const domainlist = require('./domainlist');
 const errors = require('../errors');
 const merge = require('lodash/merge');
+const isEmpty = require('lodash/isEmpty');
 const {dotize} = require('./utils');
 
 module.exports = class AssetsService {
@@ -40,7 +41,7 @@ module.exports = class AssetsService {
     ]);
 
     // If the asset exists and has settings then return the merged object.
-    if (asset && asset.settings) {
+    if (asset && asset.settings && !isEmpty(asset.settings)) {
       settings = merge({}, globalSettings, asset.settings);
     } else {
       settings = globalSettings;
@@ -95,17 +96,38 @@ module.exports = class AssetsService {
 
   /**
    * Updates the settings for the asset.
-   * @param  {[type]} id       [description]
-   * @param  {[type]} settings [description]
-   * @return {[type]}          [description]
+   * @param  {String} id        id of asset
+   * @param  {Object} settings  new settings values
+   * @return {Promise}
    */
-  static overrideSettings(id, settings) {
-    console.log(settings, dotize({settings}));
-    return AssetModel.findOneAndUpdate({id}, {
-      $set: dotize({settings})
-    }, {
-      new: true
-    });
+  static async overrideSettings(id, settings) {
+    try {
+      const result =  await AssetModel.findOneAndUpdate({id}, {
+
+        // The effect of dotize is that only the provided setting values are overwritten
+        // and does not replace the whole object.
+        $set: dotize({settings})
+      }, {
+        new: true
+      });
+      return result;
+    } catch (e) {
+
+      // Legacy data models contains `settings=null` as a default which cannot be traversed.
+      // New data models uses `settings={}`.
+      if (e.code === 16837) {
+
+        // Overwrite it fully in this case.
+        const result =  await AssetModel.findOneAndUpdate({id}, {
+          $set: {settings}
+        }, {
+          new: true
+        });
+        return result;
+      } else {
+        throw e;
+      }
+    }
   }
 
   /**
diff --git a/test/server/services/assets.js b/test/server/services/assets.js
index 6ab33321b..163210b65 100644
--- a/test/server/services/assets.js
+++ b/test/server/services/assets.js
@@ -91,7 +91,7 @@ describe('services.AssetsService', () => {
         .findOrCreateByUrl('https://override.test.com/asset')
         .then((asset) => {
           expect(asset).to.have.property('settings');
-          expect(asset.settings).to.be.null;
+          expect(asset.settings).to.be.empty;
 
           return AssetsService.overrideSettings(asset.id, {moderation: 'PRE'});
         })