From 6ef79534e4c91acaae5b9e0e9eaadb3a9efdd15a Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Thu, 21 Apr 2016 16:37:52 +0100
Subject: [PATCH] Subscribers: router & form helpers

Form:
- add confirm, location & referrer hidden fields
- add script to populate location & referrer
- add helper for creating the email field
- pass through input class and placeholder for email from top level form helper
- rename subscribe_form template & helper as it sounds more natural
- handle success and error cases differently
- improve error message display
- ensure useful data is passed back so that we can show nice messages
- check for honeypot value being filled out
- refactor error handler to set an error and always still render
---
 .../components/modals/import-subscribers.hbs  |  2 -
 core/server/api/subscribers.js                | 23 ++++---
 core/server/apps/subscribers/index.js         | 68 ++++++++++++++++---
 core/server/apps/subscribers/lib/router.js    | 61 ++++++++++++++---
 .../apps/subscribers/lib/views/subscribe.hbs  | 40 +++++++----
 core/server/helpers/index.js                  |  2 +
 core/server/helpers/input_email.js            | 43 ++++++++++++
 core/server/helpers/tpl/form_subscribe.hbs    |  8 ---
 core/server/helpers/tpl/subscribe_form.hbs    | 15 ++++
 core/server/middleware/theme-handler.js       |  3 +-
 core/server/translations/en.json              |  3 +
 11 files changed, 217 insertions(+), 51 deletions(-)
 create mode 100644 core/server/helpers/input_email.js
 delete mode 100644 core/server/helpers/tpl/form_subscribe.hbs
 create mode 100644 core/server/helpers/tpl/subscribe_form.hbs

diff --git a/core/client/app/templates/components/modals/import-subscribers.hbs b/core/client/app/templates/components/modals/import-subscribers.hbs
index a46e193355..e04b2aa9c7 100644
--- a/core/client/app/templates/components/modals/import-subscribers.hbs
+++ b/core/client/app/templates/components/modals/import-subscribers.hbs
@@ -37,8 +37,6 @@
             uploadStarted=(action 'uploadStarted')
             uploadFinished=(action 'uploadFinished')
             uploadSuccess=(action 'uploadSuccess')}}
-
-        <span>Ensure the CSV has a column named <code>email</code>.</span>
     {{/liquid-if}}
 </div>
 
diff --git a/core/server/api/subscribers.js b/core/server/api/subscribers.js
index 9213319375..6e765fdbf2 100644
--- a/core/server/api/subscribers.js
+++ b/core/server/api/subscribers.js
@@ -297,15 +297,22 @@ subscribers = {
                     var dataToImport = line.split(',');
 
                     if (firstLine) {
-                        emailIdx = _.findIndex(dataToImport, function (columnName) {
-                            if (columnName.match(/email/g)) {
-                                return true;
-                            } else {
-                                return false;
-                            }
-                        });
+                        if (dataToImport.length === 1) {
+                            emailIdx = 0;
+                        } else {
+                            emailIdx = _.findIndex(dataToImport, function (columnName) {
+                                if (columnName.match(/email/g)) {
+                                    return true;
+                                } else {
+                                    return false;
+                                }
+                            });
+                        }
+
                         if (emailIdx === -1) {
-                            return reject(new errors.ValidationError('Email column not found'));
+                            return reject(new errors.ValidationError(
+                                'Couldn\'t find your email addresses! Please use a column header which contains the word "email".'
+                            ));
                         }
                         firstLine = false;
                     } else if (emailIdx > -1) {
diff --git a/core/server/apps/subscribers/index.js b/core/server/apps/subscribers/index.js
index 3ed544e554..4199d8c9bc 100644
--- a/core/server/apps/subscribers/index.js
+++ b/core/server/apps/subscribers/index.js
@@ -1,23 +1,73 @@
 var _          = require('lodash'),
     path       = require('path'),
-    config     = require('../../config'),
+    hbs        = require('express-hbs'),
     router     = require('./lib/router'),
 
-    // Dirty require
-    template   = require('../../helpers/template');
+    // Dirty requires
+    config     = require('../../config'),
+    errors     = require('../../errors'),
+    i18n       = require('../../i18n'),
+    labs       = require('../../utils/labs'),
+    template   = require('../../helpers/template'),
+    utils      = require('../../helpers/utils'),
+
+    params = ['error', 'success', 'email', 'referrer', 'location'],
+
+    /**
+     * Dirrrrrty script
+     * <script type="text/javascript">
+     *   document.querySelector('.location').setAttribute('value', window.location.href);
+     *   document.querySelector('.referrer').setAttribute('value', document.referrer);
+     * </script>
+     */
+    subscribeScript = '<script type="text/javascript">(function(g,h,o,s,t){' +
+    'h[o](\'.location\')[s]=g.location.href;h[o](\'.referrer\')[s]=h.referrer;' +
+    '})(window,document,\'querySelector\',\'value\');</script>';
+
+function makeHidden(name) {
+    return utils.inputTemplate({
+        type: 'hidden',
+        name: name,
+        className: name,
+        extras: ''
+    });
+}
+
+function subscribeFormHelper(options) {
+    var data = _.merge({}, options.hash, _.pick(options.data.root, params), {
+        action: path.join('/', config.paths.subdir, config.routeKeywords.subscribe, '/'),
+        script: new hbs.handlebars.SafeString(subscribeScript),
+        hidden: new hbs.handlebars.SafeString(makeHidden('confirm') + makeHidden('location') + makeHidden('referrer'))
+    });
+    return template.execute('subscribe_form', data, options);
+}
 
 module.exports = {
     activate: function activate(ghost) {
+        var errorMessages = [
+            i18n.t('warnings.helpers.helperNotAvailable', {helperName: 'subscribe_form'}),
+            i18n.t('warnings.helpers.apiMustBeEnabled', {helperName: 'subscribe_form', flagName: 'subscribers'}),
+            i18n.t('warnings.helpers.seeLink', {url: 'http://support.ghost.org/subscribers-beta/'})
+        ];
+
         // Correct way to register a helper from an app
-        ghost.helpers.register('form_subscribe', function formSubscribeHelper(options) {
-            var data = _.merge({}, options.hash, {
-                action: path.join('/', config.paths.subdir, config.routeKeywords.subscribe, '/')
-            });
-            return template.execute('form_subscribe', data, options);
+        ghost.helpers.register('subscribe_form', function labsEnabledHelper() {
+            if (labs.isSet('subscribers') === true) {
+                return subscribeFormHelper.apply(this, arguments);
+            }
+
+            errors.logError.apply(this, errorMessages);
+            return new hbs.handlebars.SafeString('<script>console.error("' + errorMessages.join(' ') + '");</script>');
         });
     },
 
     setupRoutes: function setupRoutes(blogRouter) {
-        blogRouter.use('/' + config.routeKeywords.subscribe + '/', router);
+        blogRouter.use('/' + config.routeKeywords.subscribe + '/', function labsEnabledRouter(req, res, next) {
+            if (labs.isSet('subscribers') === true) {
+                return router.apply(this, arguments);
+            }
+
+            next();
+        });
     }
 };
diff --git a/core/server/apps/subscribers/lib/router.js b/core/server/apps/subscribers/lib/router.js
index 8fe8ef2afa..301677b270 100644
--- a/core/server/apps/subscribers/lib/router.js
+++ b/core/server/apps/subscribers/lib/router.js
@@ -1,18 +1,16 @@
 var path                = require('path'),
     express             = require('express'),
-    templates           = require('../../../controllers/frontend/templates'),
-    setResponseContext  = require('../../../controllers/frontend/context'),
+    subscribeRouter     = express.Router(),
+
+    // Dirty requires
     api                 = require('../../../api'),
-    subscribeRouter     = express.Router();
+    templates           = require('../../../controllers/frontend/templates'),
+    setResponseContext  = require('../../../controllers/frontend/context');
 
 function controller(req, res) {
     var defaultView = path.resolve(__dirname, 'views', 'subscribe.hbs'),
         paths = templates.getActiveThemePaths(req.app.get('activeTheme')),
-        data = {};
-
-    if (res.error) {
-        data.error = res.error;
-    }
+        data = req.body;
 
     setResponseContext(req, res);
     if (paths.hasOwnProperty('subscribe.hbs')) {
@@ -22,11 +20,47 @@ function controller(req, res) {
     }
 }
 
+function errorHandler(error, req, res, next) {
+    /*jshint unused:false */
+
+    if (error.statusCode !== 404) {
+        res.locals.error = error;
+        return controller(req, res);
+    }
+
+    next(error);
+}
+
+function honeyPot(req, res, next) {
+    if (!req.body.hasOwnProperty('confirm') || req.body.confirm !== '') {
+        return next(new Error('Oops, something went wrong!'));
+    }
+
+    // we don't need this anymore
+    delete req.body.confirm;
+    next();
+}
+
+function handleSource(req, res, next) {
+    req.body.subscribed_url = req.body.location;
+    req.body.subscribed_referrer = req.body.referrer;
+    delete req.body.location;
+    delete req.body.referrer;
+    // do something here to get post_id
+    next();
+}
 
 function storeSubscriber(req, res, next) {
-    return api.subscribers.add({subscribers: [req.body]}, {context: {external: true}}).then(function (result) {
-        next();
-    });
+    req.body.status = 'subscribed';
+
+    return api.subscribers.add({subscribers: [req.body]}, {context: {external: true}})
+        .then(function () {
+            res.locals.success = true;
+            next();
+        })
+        .catch(function (error) {
+            next(error);
+        });
 }
 
 // subscribe frontend route
@@ -35,9 +69,14 @@ subscribeRouter.route('/')
         controller
     )
     .post(
+        honeyPot,
+        handleSource,
         storeSubscriber,
         controller
     );
 
+// configure an error handler just for subscribe problems
+subscribeRouter.use(errorHandler);
+
 module.exports = subscribeRouter;
 module.exports.controller = controller;
diff --git a/core/server/apps/subscribers/lib/views/subscribe.hbs b/core/server/apps/subscribers/lib/views/subscribe.hbs
index ee018d603c..84ca7c12e1 100644
--- a/core/server/apps/subscribers/lib/views/subscribe.hbs
+++ b/core/server/apps/subscribers/lib/views/subscribe.hbs
@@ -23,24 +23,40 @@
     <div class="gh-viewport">
         <main class="gh-main" role="main">
             <div class="gh-flow">
+                <header class="gh-flow-head">
+                    <nav class="gh-flow-nav">
+                        <a href="{{@blog.url}}" class="gh-flow-back"><i class="icon-arrow-left"></i> Back</a>
+                    </nav>
+                </header>
+
                 <div class="gh-flow-content-wrap">
                     <section class="gh-flow-content">
-                        <header>
-                            <h1>Subscribe</h1>
-                        </header>
-                        {{test}}
-                        {{form_subscribe class="gh-signin"}}
+                        {{^if success}}
+                            <header>
+                                <h1>Subscribe to {{@blog.title}}</h1>
+                            </header>
+
+                            {{subscribe_form
+                                form_class="gh-signin"
+                                input_class="gh-input"
+                                button_class="btn btn-blue btn-block"
+                                placeholder="Your email address"
+                                autofocus="true"
+                            }}
+                        {{else}}
+                            <header>
+                                <h1>Subscribed!</h1>
+                            </header>
+
+                            <p>
+                                You've successfully subscribed to <em>{{@blog.title}}</em>
+                                with the email address <em>{{email}}</em>.
+                            </p>
+                        {{/if}}
                     </section>
                 </div>
             </div>
         </main>
-        {{#if error}}
-            <aside class="gh-notifications">
-                <article class="gh-notification gh-notification-red">
-                    <div id="gh-notification-content">{{error.message}}</div>
-                </article>
-            </aside>
-        {{/if}}
     </div>
 </div>
 </body>
diff --git a/core/server/helpers/index.js b/core/server/helpers/index.js
index 4df50b0b35..f806558a3a 100644
--- a/core/server/helpers/index.js
+++ b/core/server/helpers/index.js
@@ -38,6 +38,7 @@ coreHelpers.url = require('./url');
 
 // Specialist helpers for certain templates
 coreHelpers.input_password = require('./input_password');
+coreHelpers.input_email = require('./input_email');
 coreHelpers.page_url = require('./page_url');
 coreHelpers.pageUrl = require('./page_url').deprecated;
 
@@ -97,6 +98,7 @@ registerHelpers = function (adminHbs) {
     registerThemeHelper('has', coreHelpers.has);
     registerThemeHelper('is', coreHelpers.is);
     registerThemeHelper('image', coreHelpers.image);
+    registerThemeHelper('input_email', coreHelpers.input_email);
     registerThemeHelper('input_password', coreHelpers.input_password);
     registerThemeHelper('meta_description', coreHelpers.meta_description);
     registerThemeHelper('meta_title', coreHelpers.meta_title);
diff --git a/core/server/helpers/input_email.js b/core/server/helpers/input_email.js
new file mode 100644
index 0000000000..a6298a195e
--- /dev/null
+++ b/core/server/helpers/input_email.js
@@ -0,0 +1,43 @@
+// # Input Email Helper
+// Usage: `{{input_email}}`
+//
+// Password input used on private.hbs for password-protected blogs
+//
+// We use the name meta_title to match the helper for consistency:
+// jscs:disable requireCamelCaseOrUpperCaseIdentifiers
+
+var hbs             = require('express-hbs'),
+    utils           = require('./utils'),
+    input_email;
+
+input_email = function (options) {
+    options = options || {};
+    options.hash = options.hash || {};
+
+    var className = (options.hash.class) ? options.hash.class : 'subscribe-email',
+        extras = '',
+        output;
+
+    if (options.hash.autofocus) {
+        extras += 'autofocus="autofocus"';
+    }
+
+    if (options.hash.placeholder) {
+        extras += ' placeholder="' + options.hash.placeholder + '"';
+    }
+
+    if (options.hash.value) {
+        extras += ' value="' + options.hash.value + '"';
+    }
+
+    output = utils.inputTemplate({
+        type: 'email',
+        name: 'email',
+        className: className,
+        extras: extras
+    });
+
+    return new hbs.handlebars.SafeString(output);
+};
+
+module.exports = input_email;
diff --git a/core/server/helpers/tpl/form_subscribe.hbs b/core/server/helpers/tpl/form_subscribe.hbs
deleted file mode 100644
index ef97d68867..0000000000
--- a/core/server/helpers/tpl/form_subscribe.hbs
+++ /dev/null
@@ -1,8 +0,0 @@
-<form method="post" action="{{action}}" class="{{class}}">
-    <div class="form-group">
-        <span class="input-icon icon-mail">
-            <input type="email" name="email" class="gh-input">
-        </span>
-    </div>
-    <button class="btn btn-blue btn-block" type="submit">Subscribe</button>
-</form>
\ No newline at end of file
diff --git a/core/server/helpers/tpl/subscribe_form.hbs b/core/server/helpers/tpl/subscribe_form.hbs
new file mode 100644
index 0000000000..ce32b668fb
--- /dev/null
+++ b/core/server/helpers/tpl/subscribe_form.hbs
@@ -0,0 +1,15 @@
+<form method="post" action="{{action}}" class="{{form_class}}">
+    {{! This is required for the form to work correctly }}
+    {{hidden}}
+
+    <div class="form-group{{#if error}} error{{/if}}">
+        {{input_email class=input_class placeholder=placeholder value=email autofocus=autofocus}}
+    </div>
+    <button class="{{button_class}}" type="submit">Subscribe</button>
+    {{! This is used to get extra info about where this subscriber came from }}
+    {{script}}
+</form>
+
+{{#if error}}
+    <p class="main-error">{{{error.message}}}</p>
+{{/if}}
diff --git a/core/server/middleware/theme-handler.js b/core/server/middleware/theme-handler.js
index 5a914688df..5c065a14fb 100644
--- a/core/server/middleware/theme-handler.js
+++ b/core/server/middleware/theme-handler.js
@@ -27,6 +27,7 @@ themeHandler = {
     // Setup handlebars for the current context (admin or theme)
     configHbsForContext: function configHbsForContext(req, res, next) {
         var themeData = _.cloneDeep(config.theme),
+            labsData = _.cloneDeep(config.labs),
             blogApp = req.app;
 
         if (req.secure && config.urlSSL) {
@@ -38,7 +39,7 @@ themeHandler = {
         themeData.posts_per_page = themeData.postsPerPage;
         delete themeData.postsPerPage;
 
-        hbs.updateTemplateOptions({data: {blog: themeData}});
+        hbs.updateTemplateOptions({data: {blog: themeData, labs: labsData}});
 
         if (config.paths.themePath && blogApp.get('activeTheme')) {
             blogApp.set('views', path.join(config.paths.themePath, blogApp.get('activeTheme')));
diff --git a/core/server/translations/en.json b/core/server/translations/en.json
index e1c7c774cd..c7145e8c75 100644
--- a/core/server/translations/en.json
+++ b/core/server/translations/en.json
@@ -450,6 +450,9 @@
             "unableToSendEmail": "Ghost is currently unable to send email."
         },
         "helpers": {
+            "helperNotAvailable": "The \\{\\{{helperName}\\}\\} helper is not available.",
+            "apiMustBeEnabled": "The {flagName} labs flag must be enabled if you wish to use the \\{\\{{helperName}\\}\\} helper.",
+            "seeLink": "See {url}",
             "foreach": {
                 "iteratorNeeded": "Need to pass an iterator to #foreach"
             },