🎨 Split asset helper for admin & theme

refs TryGhost#7488, TryGhost#7491

- At the moment, we use the same asset helper function for both admin and theme
- Both functions are (rightly or wrongly) a wrapper around meta.getAssetUrl()
- The usecases in these contexts are v. different:
    - asset for the admin is used only in the generated index.html file
    - ghost="true" is always true in this case, except for the favicon which is ALREADY a separate case
    - minifyInProduction is only used to the admin panel, not documented or supposed to be used for themes
- For themes, the helper doesn't take any arguments, it should just call to getAssetUrl()
  If we do want to introduce some sort of .min path adjuster we should give it a better name!
- For the admin panel, minifyInProduction means exactly what it says - minify if env === production,
  it makes no sense IMO to move this to config because we control it and also plus "minifyAssets" made it
  sound like we had an asset pipeline when all this did was change the output from .js to .min.js or .css to .min.css

core/server/admin/app.js

@@ -1,7 +1,7 @@
 var debug = require('debug')('ghost:admin'),
     config = require('../config'),
     express = require('express'),
-    adminHbs = require('express-hbs').create(),
+    adminHbs = require('./handlebars'),
 
     // Admin only middleware
     redirectToSetup = require('../middleware/redirect-to-setup'),
@@ -31,9 +31,7 @@ module.exports = function setupAdminApp() {
     // Create a hbs instance for admin and init view engine
     adminApp.set('view engine', 'hbs');
     adminApp.set('views', config.get('paths').adminViews);
-    adminApp.engine('hbs', adminHbs.express3({}));
-    // Register our `asset` helper
-    adminHbs.registerHelper('asset', require('../helpers/asset'));
+    adminApp.engine('hbs', adminHbs.engine());
 
     // Admin assets
     // @TODO ensure this gets a local 404 error handler

core/server/admin/handlebars.js

@@ -0,0 +1,34 @@
+// #  Admin Asset helper
+// Usage: `{{asset "css/screen.css"}}`, `{{asset "css/screen.css" ghost="true"}}`
+//
+// Returns the path to the specified asset. The ghost flag outputs the asset path for the Ghost admin
+
+var config = require('../config'),
+    getAssetUrl = require('../data/meta/asset_url'),
+    hbs = require('express-hbs'),
+    adminHbs = hbs.create(),
+    helpers = {};
+
+helpers.adminAsset = function adminAsset(path, options) {
+    // This helper is only ever used in admin
+    var isAdmin = true,
+        minify = false;
+
+    if (options && options.hash && options.hash.minifyInProduction) {
+        // we deliberately use config.get('env') here because the setting is referring explicitly to production
+        // this is exclusively for the admin panel where we control the logic, not config
+        minify = config.get('env') === 'production';
+    }
+
+    return new hbs.handlebars.SafeString(
+        getAssetUrl(path, isAdmin, minify)
+    );
+};
+
+module.exports.engine = function setupHandlebars() {
+    adminHbs.registerHelper('asset', helpers.adminAsset);
+    return adminHbs.express3();
+};
+
+// exported for tests only
+module.exports._helpers = helpers;

core/server/config/env/config.development.json

@@ -18,7 +18,6 @@
         "useRpcPing": false,
         "useUpdateCheck": true
     },
-    "minifyAssets": false,
     "printErrorStack": true,
     "caching": {
         "theme": {

core/server/config/env/config.testing-mysql.json

@@ -52,6 +52,5 @@
     "privacy": {
         "useTinfoil": true,
         "useStructuredData": true
-    },
-    "minifyAssets": false
+    }
 }

core/server/config/env/config.testing.json

@@ -51,6 +51,5 @@
     "privacy": {
         "useTinfoil": true,
         "useStructuredData": true
-    },
-    "minifyAssets": false
+    }
 }

core/server/helpers/asset.js

@@ -1,27 +1,14 @@
 // # Asset helper
-// Usage: `{{asset "css/screen.css"}}`, `{{asset "css/screen.css" ghost="true"}}`
+// Usage: `{{asset "css/screen.css"}}`, `{{asset "css/screen.css"}}`
 //
-// Returns the path to the specified asset. The ghost flag outputs the asset path for the Ghost admin
+// Returns the path to the specified asset.
 
-var config = require('../config'),
-    getAssetUrl = require('../data/meta/asset_url'),
+var getAssetUrl = require('../data/meta/asset_url'),
     hbs = require('express-hbs');
 
-function asset(path, options) {
-    var isAdmin,
-        minify;
-
-    if (options && options.hash) {
-        isAdmin = options.hash.ghost;
-        minify = options.hash.minifyInProduction;
-    }
-
-    if (config.get('minifyAssets') === false) {
-        minify = false;
-    }
-
+function asset(path) {
     return new hbs.handlebars.SafeString(
-        getAssetUrl(path, isAdmin, minify)
+        getAssetUrl(path)
     );
 }
 

core/test/unit/admin_handlebars_spec.js

@@ -0,0 +1,105 @@
+var should         = require('should'),
+    sinon          = require('sinon'),
+    configUtils    = require('../utils/configUtils'),
+    settingsCache  = require('../../server/settings/cache'),
+
+    adminHbs       = require('../../server/admin/handlebars'),
+    helpers        = adminHbs._helpers,
+
+    sandbox        = sinon.sandbox.create();
+
+describe('ADMIN {{asset}} helper', function () {
+    var rendered, localSettingsCache = {};
+
+    before(function () {
+        configUtils.set({assetHash: 'abc'});
+
+        sandbox.stub(settingsCache, 'get', function (key) {
+            return localSettingsCache[key];
+        });
+    });
+
+    after(function () {
+        configUtils.restore();
+        sandbox.restore();
+    });
+
+    describe('no subdirectory', function () {
+        it('handles favicon correctly', function () {
+            rendered = helpers.adminAsset('favicon.ico');
+            should.exist(rendered);
+            String(rendered).should.equal('/favicon.ico');
+        });
+
+        it('handles custom favicon correctly', function () {
+            localSettingsCache.icon = '/content/images/favicon.png';
+
+            // png
+            rendered = helpers.adminAsset('favicon.png');
+            should.exist(rendered);
+            String(rendered).should.equal('/favicon.ico');
+
+            localSettingsCache.icon = '/content/images/favicon.ico';
+
+            // ico
+            rendered = helpers.adminAsset('favicon.ico');
+            should.exist(rendered);
+        });
+
+        it('handles shared assets correctly', function () {
+            localSettingsCache.icon = '';
+
+            rendered = helpers.adminAsset('shared/asset.js');
+            should.exist(rendered);
+            String(rendered).should.equal('/shared/asset.js?v=abc');
+        });
+
+        it('handles admin assets correctly', function () {
+            rendered = helpers.adminAsset('js/asset.js');
+            should.exist(rendered);
+            String(rendered).should.equal('/ghost/assets/js/asset.js?v=abc');
+        });
+    });
+
+    describe('with /blog subdirectory', function () {
+        before(function () {
+            configUtils.set({url: 'http://testurl.com/blog'});
+        });
+
+        it('handles favicon correctly', function () {
+            rendered = helpers.adminAsset('favicon.ico');
+            should.exist(rendered);
+            String(rendered).should.equal('/blog/favicon.ico');
+        });
+
+        it('handles custom favicon correctly', function () {
+            localSettingsCache.icon = '/content/images/favicon.png';
+
+            // png
+            rendered = helpers.adminAsset('favicon.png');
+            should.exist(rendered);
+            String(rendered).should.equal('/blog/favicon.ico');
+
+            localSettingsCache.icon = '/content/images/favicon.ico';
+
+            // ico
+            rendered = helpers.adminAsset('favicon.ico');
+            should.exist(rendered);
+            String(rendered).should.equal('/blog/favicon.ico');
+        });
+
+        it('handles shared assets correctly', function () {
+            rendered = helpers.adminAsset('shared/asset.js');
+            should.exist(rendered);
+            String(rendered).should.equal('/blog/shared/asset.js?v=abc');
+        });
+
+        it('handles admin assets correctly', function () {
+            rendered = helpers.adminAsset('js/asset.js');
+            should.exist(rendered);
+            String(rendered).should.equal('/blog/ghost/assets/js/asset.js?v=abc');
+        });
+
+        configUtils.restore();
+    });
+});

core/test/unit/server_helpers/asset_spec.js

@@ -31,11 +31,6 @@ describe('{{asset}} helper', function () {
 
     describe('no subdirectory', function () {
         it('handles favicon correctly', function () {
-            // with ghost set
-            rendered = helpers.asset('favicon.ico', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/favicon.ico');
-
             // without ghost set
             rendered = helpers.asset('favicon.ico');
             should.exist(rendered);
@@ -45,24 +40,14 @@ describe('{{asset}} helper', function () {
         it('handles custom favicon correctly', function () {
             localSettingsCache.icon = '/content/images/favicon.png';
 
-            // with ghost set and png
-            rendered = helpers.asset('favicon.png', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/favicon.ico');
-
-            // without ghost set and png
+            // png
             rendered = helpers.asset('favicon.png');
             should.exist(rendered);
             String(rendered).should.equal('/content/images/favicon.png');
 
             localSettingsCache.icon = '/content/images/favicon.ico';
 
-            // with ghost set and ico
-            rendered = helpers.asset('favicon.ico', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/favicon.ico');
-
-            // without ghost set and ico
+            // ico
             rendered = helpers.asset('favicon.ico');
             should.exist(rendered);
             String(rendered).should.equal('/content/images/favicon.ico');
@@ -71,26 +56,12 @@ describe('{{asset}} helper', function () {
         it('handles shared assets correctly', function () {
             localSettingsCache.icon = '';
 
-            // with ghost set
-            rendered = helpers.asset('shared/asset.js', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/shared/asset.js?v=abc');
-
-            // without ghost set
             rendered = helpers.asset('shared/asset.js');
             should.exist(rendered);
             String(rendered).should.equal('/shared/asset.js?v=abc');
         });
 
-        it('handles admin assets correctly', function () {
-            // with ghost set
-            rendered = helpers.asset('js/asset.js', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/ghost/assets/js/asset.js?v=abc');
-        });
-
         it('handles theme assets correctly', function () {
-            // with ghost set
             rendered = helpers.asset('js/asset.js');
             should.exist(rendered);
             String(rendered).should.equal('/assets/js/asset.js?v=abc');
@@ -103,12 +74,6 @@ describe('{{asset}} helper', function () {
         });
 
         it('handles favicon correctly', function () {
-            // with ghost set
-            rendered = helpers.asset('favicon.ico', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/blog/favicon.ico');
-
-            // without ghost set
             rendered = helpers.asset('favicon.ico');
             should.exist(rendered);
             String(rendered).should.equal('/blog/favicon.ico');
@@ -117,50 +82,26 @@ describe('{{asset}} helper', function () {
         it('handles custom favicon correctly', function () {
             localSettingsCache.icon = '/content/images/favicon.png';
 
-            // with ghost set and png
-            rendered = helpers.asset('favicon.png', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/blog/favicon.ico');
-
-            // without ghost set and png
+            // png
             rendered = helpers.asset('favicon.png');
             should.exist(rendered);
             String(rendered).should.equal('/blog/content/images/favicon.png');
 
             localSettingsCache.icon = '/content/images/favicon.ico';
 
-            // with ghost set and ico
-            rendered = helpers.asset('favicon.ico', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/blog/favicon.ico');
-
-            // without ghost set and ico
+            // ico
             rendered = helpers.asset('favicon.ico');
             should.exist(rendered);
             String(rendered).should.equal('/blog/content/images/favicon.ico');
         });
 
         it('handles shared assets correctly', function () {
-            // with ghost set
-            rendered = helpers.asset('shared/asset.js', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/blog/shared/asset.js?v=abc');
-
-            // without ghost set
             rendered = helpers.asset('shared/asset.js');
             should.exist(rendered);
             String(rendered).should.equal('/blog/shared/asset.js?v=abc');
         });
 
-        it('handles admin assets correctly', function () {
-            // with ghost set
-            rendered = helpers.asset('js/asset.js', {hash: {ghost: 'true'}});
-            should.exist(rendered);
-            String(rendered).should.equal('/blog/ghost/assets/js/asset.js?v=abc');
-        });
-
         it('handles theme assets correctly', function () {
-            // with ghost set
             rendered = helpers.asset('js/asset.js');
             should.exist(rendered);
             String(rendered).should.equal('/blog/assets/js/asset.js?v=abc');