🎨 tidy up static id (owner, internal, external) usages (#7675)

refs #7494, refs #7495 

This PR is an extracted clean up feature of #7495.
We are using everywhere static id checks (userId === 0 or userId === 1).
This PR moves the static values into the Base model.
This makes it 1. way more readable and 2. we can change the id's in a central place.

I changed the most important occurrences - no tests are touched (yet!).

The background is: when changing from auto increment id (number) to ObjectId's (string) we still need to support id 1 and 0, because Ghost relies on these two static id's.
I would like to support using both: 0/1 as string and 0/1 as number.

1 === owner/internal
0 === external

Another important change:
User Model does not longer define the contextUser method, because i couldn't find a reason?
I looked in Git history, see 6e48275160

core/server/api/index.js

@@ -7,6 +7,7 @@
 var _              = require('lodash'),
     Promise        = require('bluebird'),
     config         = require('../config'),
+    models         = require('../models'),
     utils          = require('../utils'),
     configuration  = require('./configuration'),
     db             = require('./db'),
@@ -220,7 +221,7 @@ http = function http(apiMethod) {
             options = _.extend({}, req.file, req.query, req.params, {
                 context: {
                     // @TODO: forward the client and user obj in 1.0 (options.context.user.id)
-                    user: ((req.user && req.user.id) || (req.user && req.user.id === 0)) ? req.user.id : null,
+                    user: ((req.user && req.user.id) || (req.user && models.User.isExternalUser(req.user.id))) ? req.user.id : null,
                     client: (req.client && req.client.slug) ? req.client.slug : null,
                     client_id: (req.client && req.client.id) ? req.client.id : null
                 }

core/server/auth/authenticate.js

@@ -1,5 +1,6 @@
 var passport = require('passport'),
     errors = require('../errors'),
+    models = require('../models'),
     events = require('../events'),
     i18n = require('../i18n'),
     authenticate;
@@ -96,7 +97,7 @@ authenticate = {
                         message: i18n.t('errors.middleware.auth.accessDenied')
                     }));
                 } else if (req.client) {
-                    req.user = {id: 0};
+                    req.user = {id: models.User.externalUser};
                     return next();
                 }
 

core/server/data/import/utils.js

@@ -73,13 +73,11 @@ utils = {
             if (foundUser && _.has(foundUser, 'email') && _.has(existingUsers, foundUser.email)) {
                 existingUsers[foundUser.email].importId = userToMap;
                 userMap[userToMap] = existingUsers[foundUser.email].realId;
-            } else if (userToMap === 1) {
-                // if we don't have user data and the id is 1, we assume this means the owner
+            } else if (models.User.isOwnerUser(userToMap)) {
                 existingUsers[owner.email].importId = userToMap;
                 userMap[userToMap] = existingUsers[owner.email].realId;
-            } else if (userToMap === 0) {
-                // CASE: external context
-                userMap[userToMap] = '0';
+            } else if (models.User.isExternalUser(userToMap)) {
+                userMap[userToMap] = models.User.externalUser;
             } else {
                 throw new errors.DataImportError({
                     message: i18n.t('errors.data.import.utils.dataLinkedToUnknownUser', {userToMap: userToMap}),

core/server/models/base/index.js

@@ -179,14 +179,17 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
 
     // Get the user from the options object
     contextUser: function contextUser(options) {
-        // Default to context user
-        if ((options.context && options.context.user) || (options.context && options.context.user === 0)) {
+        options = options || {};
+        options.context = options.context || {};
+
+        if (_.isNumber(options.context.user)) {
             return options.context.user;
-        // Other wise use the internal override
-        } else if (options.context && options.context.internal) {
-            return 1;
-        } else if (options.context && options.context.external) {
-            return 0;
+        } else if (options.context.internal) {
+            return ghostBookshelf.Model.internalUser;
+        } else if (this.get('id')) {
+            return this.get('id');
+        } else if (options.context.external) {
+            return ghostBookshelf.Model.externalUser;
         } else {
             throw new errors.NotFoundError({
                 message: i18n.t('errors.models.base.index.missingContext'),
@@ -249,6 +252,25 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
 }, {
     // ## Data Utility Functions
 
+    /**
+     * please use these static definitions when comparing id's
+     */
+    internalUser: 1,
+    ownerUser: 1,
+    externalUser: 0,
+
+    isOwnerUser: function isOwnerUser(id) {
+        return id === ghostBookshelf.Model.ownerUser;
+    },
+
+    isInternalUser: function isInternalUser(id) {
+        return id === ghostBookshelf.Model.internalUser;
+    },
+
+    isExternalUser: function isExternalUser(id) {
+        return id === ghostBookshelf.Model.externalUser;
+    },
+
     /**
      * Returns an array of keys permitted in every method's `options` hash.
      * Can be overridden and added to by a model's `permittedOptions` method.

core/server/models/user.js

@@ -162,24 +162,6 @@ User = ghostBookshelf.Model.extend({
         return validation.validateSchema(this.tableName, userData);
     },
 
-    // Get the user from the options object
-    contextUser: function contextUser(options) {
-        // Default to context user
-        if (options.context && options.context.user) {
-            return options.context.user;
-            // Other wise use the internal override
-        } else if (options.context && options.context.internal) {
-            return 1;
-            // This is the user object, so try using this user's id
-        } else if (this.get('id')) {
-            return this.get('id');
-        } else {
-            throw new errors.NotFoundError({
-                message: i18n.t('errors.models.user.missingContext')
-            });
-        }
-    },
-
     toJSON: function toJSON(options) {
         options = options || {};