From 5b9c213849eb500d71436c0203bcd0e26803fb92 Mon Sep 17 00:00:00 2001
From: Katharina Irrgang <katharina.irrgang@googlemail.com>
Date: Thu, 13 Oct 2016 14:52:22 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20=20change=20gravatar=20file=20de?=
 =?UTF-8?q?sign=20(#7553)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

no issue
- preperation for User model refactoring
- the rule is:
  --> when calling a unit, this unit should return something new
  --> and NOT modifying an existing object and return it (this is an unexpected behaviour, especially for utils and libs)
---
 core/server/models/user.js          | 23 ++++++++++++++---------
 core/server/utils/gravatar.js       | 19 +++++++++++++------
 core/test/unit/server_utils_spec.js |  4 +---
 3 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/core/server/models/user.js b/core/server/models/user.js
index eeaa136119..a241b7ef5a 100644
--- a/core/server/models/user.js
+++ b/core/server/models/user.js
@@ -407,9 +407,12 @@ User = ghostBookshelf.Model.extend({
         return generatePasswordHash(userData.password).then(function then(hash) {
             // Assign the hashed password
             userData.password = hash;
-            // LookupGravatar
             return gravatar.lookup(userData);
-        }).then(function then(userData) {
+        }).then(function then(response) {
+            if (response && response.image) {
+                userData.image = response.image;
+            }
+
             // Save the user with the hashed password
             return ghostBookshelf.Model.add.call(self, userData, options);
         }).then(function then(addedUser) {
@@ -451,14 +454,16 @@ User = ghostBookshelf.Model.extend({
             // Assign the hashed password
             userData.password = hash;
 
-            return Promise.join(
-                gravatar.lookup(userData),
-                ghostBookshelf.Model.generateSlug.call(this, User, userData.name, options)
-            );
-        }).then(function then(results) {
-            userData = results[0];
-            userData.slug = results[1];
+            return gravatar.lookup(userData)
+                .then(function (response) {
+                    if (response && response.image) {
+                        userData.image = response.image;
+                    }
 
+                    return ghostBookshelf.Model.generateSlug.call(this, User, userData.name, options);
+                });
+        }).then(function then(slug) {
+            userData.slug = slug;
             return self.edit.call(self, userData, options);
         });
     },
diff --git a/core/server/utils/gravatar.js b/core/server/utils/gravatar.js
index 106d567fa9..3662fc0d8c 100644
--- a/core/server/utils/gravatar.js
+++ b/core/server/utils/gravatar.js
@@ -6,37 +6,44 @@ var Promise = require('bluebird'),
 module.exports.lookup = function lookup(userData, timeout) {
     var gravatarUrl = '//www.gravatar.com/avatar/' +
         crypto.createHash('md5').update(userData.email.toLowerCase().trim()).digest('hex') +
-        '?s=250';
+        '?s=250', image;
 
     return new Promise(function gravatarRequest(resolve) {
+        /**
+         * @TODO:
+         *  - mock gravatar in test env globally, do not check for test env!
+         *  - in test/utils/index.js -> mocks.gravatar.enable();
+         */
         if (config.isPrivacyDisabled('useGravatar') || config.get('env').indexOf('testing') > -1) {
-            return resolve(userData);
+            return resolve();
         }
 
         var request, timer, timerEnded = false;
 
         request = https.get('https:' + gravatarUrl + '&d=404&r=x', function (response) {
             clearTimeout(timer);
+
             if (response.statusCode !== 404 && !timerEnded) {
                 gravatarUrl += '&d=mm&r=x';
-                userData.image = gravatarUrl;
+                image = gravatarUrl;
             }
 
-            resolve(userData);
+            resolve({image: image});
         });
 
         request.on('error', function () {
             clearTimeout(timer);
+
             // just resolve with no image url
             if (!timerEnded) {
-                return resolve(userData);
+                return resolve();
             }
         });
 
         timer = setTimeout(function () {
             timerEnded = true;
             request.abort();
-            return resolve(userData);
+            return resolve();
         }, timeout || 2000);
     });
 };
diff --git a/core/test/unit/server_utils_spec.js b/core/test/unit/server_utils_spec.js
index a34fae02f0..55f098428a 100644
--- a/core/test/unit/server_utils_spec.js
+++ b/core/test/unit/server_utils_spec.js
@@ -410,9 +410,7 @@ describe('Server Utilities', function () {
                 .reply(200);
 
             gravatar.lookup({email: 'exists@example.com'}, 10).then(function (result) {
-                should.exist(result);
-                should.not.exist(result.image);
-
+                should.not.exist(result);
                 done();
             }).catch(done);
         });