Use extends correctly & consistently

- extends clobbers the first argument you pass to it, so that should not be a variable that is used elsewhere, if you're also assigning the value, as it will have unintended side effects.
TryGhost · Jun 25, 2015 · f6322da · f6322da
1 parent b3f4bea
commit f6322da
Show file tree

Hide file tree

Showing 9 changed files with 17 additions and 17 deletions.
diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js
@@ -211,7 +211,7 @@ authentication = {
             return dataProvider.User.findOne({role: 'Owner', status: 'all'});
         }).then(function (ownerUser) {
             if (ownerUser) {
-                return dataProvider.User.setup(setupUser, _.extend(internal, {id: ownerUser.id}));
+                return dataProvider.User.setup(setupUser, _.extend({id: ownerUser.id}, internal));
             } else {
                 return dataProvider.Role.findOne({name: 'Owner'}).then(function (ownerRole) {
                     setupUser.roles = [ownerRole.id];

diff --git a/core/server/models/post.js b/core/server/models/post.js
@@ -401,9 +401,9 @@ Post = ghostBookshelf.Model.extend({
         var withNext = _.contains(options.include, 'next'),
             withPrev = _.contains(options.include, 'previous');
 
-        data = _.extend({
+        data = _.defaults(data || {}, {
             status: 'published'
-        }, data || {});
+        });
 
         if (data.status === 'all') {
             delete data.status;

diff --git a/core/server/models/user.js b/core/server/models/user.js
@@ -263,9 +263,9 @@ User = ghostBookshelf.Model.extend({
 
         delete data.role;
 
-        data = _.extend({
+        data = _.defaults(data || {}, {
             status: 'active'
-        }, data || {});
+        });
 
         status = data.status;
         delete data.status;

diff --git a/core/server/update-check.js b/core/server/update-check.js
@@ -56,9 +56,9 @@ function updateCheckData() {
         ops = [],
         mailConfig = config.mail;
 
-    ops.push(api.settings.read(_.extend(internal, {key: 'dbHash'})).catch(errors.rejectError));
-    ops.push(api.settings.read(_.extend(internal, {key: 'activeTheme'})).catch(errors.rejectError));
-    ops.push(api.settings.read(_.extend(internal, {key: 'activeApps'}))
+    ops.push(api.settings.read(_.extend({key: 'dbHash'}, internal)).catch(errors.rejectError));
+    ops.push(api.settings.read(_.extend({key: 'activeTheme'}, internal)).catch(errors.rejectError));
+    ops.push(api.settings.read(_.extend({key: 'activeApps'}, internal))
         .then(function (response) {
             var apps = response.settings[0];
             try {
@@ -187,7 +187,7 @@ function updateCheck() {
         // No update check
         return Promise.resolve();
     } else {
-        return api.settings.read(_.extend(internal, {key: 'nextUpdateCheck'})).then(function then(result) {
+        return api.settings.read(_.extend({key: 'nextUpdateCheck'}, internal)).then(function then(result) {
             var nextUpdateCheck = result.settings[0];
 
             if (nextUpdateCheck && nextUpdateCheck.value && nextUpdateCheck.value > moment().unix()) {
@@ -204,7 +204,7 @@ function updateCheck() {
 }
 
 function showUpdateNotification() {
-    return api.settings.read(_.extend(internal, {key: 'displayUpdateNotification'})).then(function then(response) {
+    return api.settings.read(_.extend({key: 'displayUpdateNotification'}, internal)).then(function then(response) {
         var display = response.settings[0];
 
         // Version 0.4 used boolean to indicate the need for an update. This special case is

diff --git a/core/test/integration/api/api_configuration_spec.js b/core/test/integration/api/api_configuration_spec.js
@@ -32,7 +32,7 @@ describe('Configuration API', function () {
     should.exist(ConfigurationAPI);
 
     it('can browse config', function (done) {
-        var updatedConfig = _.extend(config, newConfig);
+        var updatedConfig = _.extend({}, config, newConfig);
         config.set(updatedConfig);
         ConfigurationAPI.__set__('config', updatedConfig);
 
@@ -49,7 +49,7 @@ describe('Configuration API', function () {
     });
 
     it('can read config', function (done) {
-        var updatedConfig = _.extend(config, newConfig);
+        var updatedConfig = _.extend({}, config, newConfig);
         config.set(updatedConfig);
         ConfigurationAPI.__set__('config', updatedConfig);
 

diff --git a/core/test/integration/api/api_notifications_spec.js b/core/test/integration/api/api_notifications_spec.js
@@ -122,7 +122,7 @@ describe('Notifications API', function () {
             var notification = result.notifications[0];
 
             NotificationsAPI.destroy(
-                _.extend(testUtils.context.internal, {id: notification.id})
+                _.extend({}, testUtils.context.internal, {id: notification.id})
             ).then(function (result) {
                 should.exist(result);
                 should.exist(result.notifications);
@@ -143,7 +143,7 @@ describe('Notifications API', function () {
             var notification = result.notifications[0];
 
             NotificationsAPI.destroy(
-                _.extend(testUtils.context.owner, {id: notification.id})
+                _.extend({}, testUtils.context.owner, {id: notification.id})
             ).then(function (result) {
                 should.exist(result);
                 should.exist(result.notifications);

diff --git a/core/test/integration/import_spec.js b/core/test/integration/import_spec.js
@@ -34,7 +34,7 @@ describe('Import', function () {
     describe('Resolves', function () {
         beforeEach(testUtils.setup());
         beforeEach(function () {
-            var newConfig = _.extend(config, defaultConfig);
+            var newConfig = _.extend({}, config, defaultConfig);
 
             migration.__get__('config', newConfig);
             config.set(newConfig);

diff --git a/core/test/unit/storage_local-file-store_spec.js b/core/test/unit/storage_local-file-store_spec.js
@@ -17,7 +17,7 @@ describe('Local File System Storage', function () {
     var image,
         overrideConfig = function (newConfig) {
             var existingConfig = LocalFileStore.__get__('config'),
-                updatedConfig = _.extend(existingConfig, newConfig);
+                updatedConfig = _.extend({}, existingConfig, newConfig);
             config.set(updatedConfig);
             LocalFileStore.__set__('config', updatedConfig);
         };

diff --git a/core/test/utils/fork.js b/core/test/utils/fork.js
@@ -55,7 +55,7 @@ function forkGhost(newConfig, envName) {
         .then(function (port) {
             newConfig.server = newConfig.server || {};
             newConfig.server.port = port;
-            newConfig.url = url.format(_.extend(url.parse(newConfig.url), {port: port, host: null}));
+            newConfig.url = url.format(_.extend({}, url.parse(newConfig.url), {port: port, host: null}));
 
             var newConfigFile = path.join(config.paths.appRoot, 'config.test' + port + '.js');