Issue 6210 - Implement Subscription (Double-opt-in) template for newsletter

gulpfile.js

+/*!
+ * This file is part of eyeomail.com.
+ * Copyright (C) 2017-present eyeo GmbH
+ *
+ * eyeomail.com is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * eyeomail.com is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with eyeomail.com.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+const gulp = require("gulp");
+const gutil = require("gulp-util");
+const rename = require("gulp-rename");
+const sass = require("gulp-sass");
+const postcss = require("gulp-postcss");
+const scss = require("postcss-scss");
+const autoprefixer = require("autoprefixer");
+const replace = require("replace");
+const fs = require("fs");
+const readFilePromise = require("fs-readfile-promise");
+
+/******************************************************************************
+ * CSS
+ ******************************************************************************/
+
+gulp.task("compileCss", function() {
+  return gulp.src("./src/scss/main.scss")
+    .pipe(postcss([autoprefixer()], {syntax: scss}).on("error", gutil.log))
+    .pipe(sass({outputStyle: "compact"}).on("error", gutil.log))
+    .pipe(gulp.dest("./dist"));
+});
+
+gulp.task("inlineCss", ["compileCss", "templates"], function() {
+  return fs.readFile("./dist/main.css", "utf8", function (err, styles) {
+    if (err) throw err;
+
+    const replacement = `<!-- styles:start -->

[juliandoucette, 2018/01/22 19:47:52]

NIT: (Bringing up an old point, I know)

I understand that placing the opening and closing style tags on their own lines
makes the end result cleaner. But why replace <! -- styles:(start|end) --> with
itself (plus contents within - instead of just replacing the contents within)?
And why have an opening **and closing** comment at all if we are replacing
everything within? I understand that it may be common, but it doesn't make any
sense in this implementation.

[ire, 2018/01/23 08:48:08]

The reason I replaced the comments as well was because I found that regex easier
to implement. If you can find a regex that would just target the contents of the
two comments, let me know.
We need both in case we are trying to override styles that already exit. e.g. if
we are updating the template and not starting from a blank template that just
has the opening and closing comments, nothing in between. If we don't have the
consistent opening and closing comments, how would we know where the styles
start and stop? We could use the actual `<style>` tags in this case but I think
its more future-proof to use the comments in case we for some reason have
multiple <style> elements.
What I said was common was the format of the styles, i.e. having `styles:start`
and `styles:end`. Why doesn't that make any sense? And what do you think would
work better?

[juliandoucette, 2018/01/23 17:00:45]

I wasn't able to find a simple regex solution. My understanding of regex is
currently very limited.
 
But we are not currently, nor are we planning to, override styles that already
exist? 
<!-- styles -->, ${styles}, {{ styles }}, or similar.

This start and stop format is not necessary unless we are planning to use it to
do something more advanced e.g.

<style>
/* styles that always apply */
<!-- style:start -->
/* styles that apply to a pre-complied state and not to a post-compiled state */
<!-- style:end -->
</style>

Am I missing something?

+<style>
+${styles}
+</style>
+<!-- styles:end -->`;
+
+    replace({
+      regex: /<!-- styles:start -->([\s\S]*?)<!-- styles:end -->/,
+      replacement: replacement,
+      paths: ["dist/signup.html"],
+      recursive: true,
+      silent: true,
+    });
+  });
+});
+
+gulp.task("css", ["inlineCss"]);
+
+/******************************************************************************
+ * Template
+ ******************************************************************************/
+
+gulp.task("templates", function() {

[juliandoucette, 2018/01/22 19:47:52]

NIT: This task name is inconsistent with the ones above. If I understand it's
function correctly it should be moveTemplates (or similar)?

[ire, 2018/01/23 08:48:09]

Done.

+  const templateFiles = ["./src/templates/signup.html", 
+                         "./src/templates/confirm-email.html", 
+                         "./src/templates/subscription-confirmed-email.html"];
+
+  return gulp.src(templateFiles).pipe(gulp.dest("./dist"));
+});
+
+gulp.task("backclick", function() {
+  const templateFiles = ["./dist/signup.html", 
+                         "./dist/confirm-email.html", 
+                         "./dist/subscription-confirmed-email.html"];
+
+  return Promise.all(templateFiles.map((file) => readFilePromise(file)))
+    .then((buffers) => Promise.all(buffers.map((buffer) => buffer.toString())))
+    .then((contents) => {
+      fs.mkdir("./dist", () => {

[juliandoucette, 2018/01/22 19:47:51]

NIT: mkdir implies that a dir doesn't exist already (because the unix command
will throw an error in this case). Which looks like a problem because the
"template" task runs before the "backclick" task (files are possibly moved to a
directory before it is created). I would add a comment somewhere that explains
why this doesn't happen or use a mkdir function that doesn't  throw an error if
a directory already exists.

[ire, 2018/01/23 08:48:08]

The `mkdir` is actually not needed as the backclick task should only ever be run
after the template/css tasks. Removed

+        fs.writeFile("./dist/backclick.html", contents.join(""));

[juliandoucette, 2018/01/22 19:47:51]

NIT: A similar issue to the one above applies here. fs.writeFile implies that a
file doesn't already exist. Yet there is no "clean" (erase) task in this code.

[ire, 2018/01/23 08:48:08]

From my understanding fs.writeFile can be used to replace an existing file as
well
(https://nodejs.org/api/fs.html#fs_fs_writefile_file_data_options_callback). 

[juliandoucette, 2018/01/23 17:00:45]

Acknowledged.

+      })
+    });
+});
+
+gulp.task("template", ["css"]);
+
+/******************************************************************************
+ * Watch
+ ******************************************************************************/
+
+gulp.task("watch", function() {
+  gulp.watch("./src/scss/**/*.scss", ["css"]);
+  gulp.watch("./src/templates/*.html", ["template", "backclick"]);
+});
+
+/******************************************************************************
+ * Default
+ ******************************************************************************/
+
+gulp.task("default", ["template", "css", "watch"]);