From 30b631a7a62c90cb51d5b5f6fbf3b40ba9a89cca Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Sat, 31 Jan 2015 17:56:35 +0000 Subject: [PATCH 01/14] Fixed a couple of grammar mistakes --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e2d7f47..d9f764a 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ To contribute to discuss the specification and APIs, go to the specific issue ## Specification -- Implement a core without lock in to any email provider nor template library which fulfil [gitevents](https://github.com/GitEvents/gitevents) needs. +- Implement a core without lock in any specific email provider nor template library which fulfil [gitevents](https://github.com/GitEvents/gitevents) needs. - Send mass emails to subscribers (e.g. newsletters). - Support different template engines, we call them renders. - Support different email providers, we call them mailers; each provider must transform option parameters to the expected format and passing its expected parameters. @@ -36,7 +36,7 @@ The module API must fulfil the [gitevents](https://github.com/GitEvents/gitevent The first approach that we've thought is exporting a function which receive an options object, then it returns something that we have to define if it's a function or an object. -For this module we've thought that options object should have as a required options: +For this module we've thought that options object should have as required options: * mailer: The mailer module which implement the [Mailer API plugin](#mailer-api-plugin) * renderer: The renderer module which implement the [Renderer API plugin](#renderer-api-plugin) From a9bdd3d6cd7848c736b4d8468b83606f13742213 Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Sun, 1 Feb 2015 01:35:55 +0000 Subject: [PATCH 02/14] Fixed some incorrect plugins names --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d9f764a..c848153 100644 --- a/README.md +++ b/README.md @@ -20,10 +20,10 @@ To contribute to discuss the specification and APIs, go to the specific issue - Implement a core without lock in any specific email provider nor template library which fulfil [gitevents](https://github.com/GitEvents/gitevents) needs. - Send mass emails to subscribers (e.g. newsletters). -- Support different template engines, we call them renders. +- Support different template engines, we call them renderers. - Support different email providers, we call them mailers; each provider must transform option parameters to the expected format and passing its expected parameters. - Only offer global and generic email options; any information in the content of the email has to be set in the template with a variable, although they are recommended for example to render on any client or to reduce to get capture by the spam filter, they don't have to be in the modules, that task relies in the person who build the template, not in this module. For this reason the module will be pluggable for: - - Renders + - Renderers - Mailers - Possibility to use features offered by several providers, for example, email addresses list of the subscribers, etc. - Keep simple and modular. @@ -66,7 +66,7 @@ Send method may contain some parameters that they aren't often used, however we ## Renderer API plugin -Any __render__ plugin module must export a function which must receive the next parameters: +Any __renderer__ plugin module must export a function which must receive the next parameters: 1. `htmlTemplate {String}`: A string with the HTML version of the template to render. 2. `textTemplate {String}`: A string with the plain text version of the template to render. From ab82ae2a26afc47ae2f73b251a53f2a965519aab Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Sun, 1 Feb 2015 02:11:37 +0000 Subject: [PATCH 03/14] Fixed grammar, some sentences didn't make sense --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c848153..8dca572 100644 --- a/README.md +++ b/README.md @@ -60,9 +60,9 @@ For `send` method we've roughly thought to have the next fixed parameters: 6. `body {Object}`: Object with 2 properties: * `html {String}`: HTML version of the message's body. * `text {String}`: Plain text version of the message's body. -7. `options {Object}`: Object with variable properties which allow to user specific features of the provider, however some of them can be common to each mailer but they are not probably used they are not a function parameter, e.g. attachements +7. `options {Object}` (optional): An object with specific properties of the implemented provider. Note they are unlikely compatible between different providers, therefore if you change the mailer, then you will have to update it, moreover some mailers can ignore it. In the most of the cases, they would unlikely needed, e.g. attachments. -Send method may contain some parameters that they aren't often used, however we thought to have as a parameter because they are very common to a email send action; however to avoid the complexity to manage optional parameters moreover that make the code more difficult to understand if there are too many, then we may move them to the `options` object. +Send method may contain some parameters that they aren't often used, however we thought to have as a parameter because they are very common to any email send action; however to avoid the complexity to manage optional parameters moreover they make the code more difficult to understand, if there are too many, then we may move them to the `options` object. ## Renderer API plugin @@ -70,8 +70,8 @@ Any __renderer__ plugin module must export a function which must receive the nex 1. `htmlTemplate {String}`: A string with the HTML version of the template to render. 2. `textTemplate {String}`: A string with the plain text version of the template to render. -3. `locals {Object}`: An object which contains the template variables values to use as default none are needed then `null`. -4. config {Object} (optional)`: An object which may contains specific configurations for the template library. Note they are unlikely compatible between different renderers, hence if you change the renderer then you will have to update it moreover that renderer may ignore this parameters. +3. `locals {Object}`: An object which contains the template variables values to use as default, if none are needed then `null`. +4. `config {Object} (optional)`: An object which contains specific configurations for the template library. Note they are unlikely compatible between different renderers, hence if you change the renderer then you may have to update it, furthermore some renderer may ignore this parameters. Although __two templates types have to pass as a parameter we may only require one__, then one of them can be null, obviously the result will only contain the provided one. @@ -86,7 +86,7 @@ Any __renderer__ plugin module must export a function which must receive the nex We know that the most of the template engines render templates from files, however to keep the API simple we suggest to only render string templates for two main reasons: 1. Render from file is something we want to do, however reading a file from a disk is something that can be done as standalone tool/util/helper and avoid complexity in renderers implementations and duplicate functionality which should be tested as well. I've also thought that email templates are quite complicated and they have a lot of pain points if they are compatible for at least the most email clients, desktop and browser ones, then keeping them as a single template without partials and layout extension (inheritance), so a single and simple string is enough. -2. Even though to render templates from files seem as a requirement for `gitevents`, we consider that it isn't because the group want to host the template in a public URL (e.g. a Github gist), then the renderer's API should have another function for templates available in an URL, then as I've commented previously the API should be more complex and probably the renders woud have duplicated functionality. +2. Even though to render templates from files seem as a requirement for `gitevents`, we consider that it isn't because the group want to host the template in a public URL (e.g. a Github gist), then the renderer's API should have another function for templates available in an URL, then as I've commented previously the API should be more complex and probably the renders would have duplicated functionality. A challenging point here is if with this interface we could implement a renderer which use remote templates hosted by the email provider, for example, using a mailchimp mailer we can implement a specific renderer that allows to the emailer to use its template API, of course mailchimp renderer would only work with mailchimp mailer, however mailchimp mailer must work as any other renderer which is not specific of an email provider. From 3c113c7e2a182d9bcfcef508847fcafaab417d1c Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Sun, 1 Feb 2015 20:11:32 +0000 Subject: [PATCH 04/14] Fixed wrong markdown link format --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8dca572..f167a82 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Mailer plugin for [gitevents](https://github.com/GitEvents/gitevents) -Clarifications this document use "we" as a community, however so far, sometime "we" may mean one person, two people or several people, then bear in mind that "we", in the time being, does not mean [GitEvents organisation][https://github.com/GitEvents] which is discussing several aspects and some of them are pushed by one of the members as a start point of a discussion. +Clarifications this document use "we" as a community, however so far, sometime "we" may mean one person, two people or several people, then bear in mind that "we", in the time being, does not mean [GitEvents organisation](https://github.com/GitEvents) which is discussing several aspects and some of them are pushed by one of the members as a start point of a discussion. __NOTE__: This module is very early stage of development. This README contains several sections which express what it should be, however we encourage people who may interested in [gitevents](https://github.com/GitEvents/gitevents) project to contribute to define the specification, the module's API, etc. From 62d29c58e5742349a0ad7668e6bc35137efac37e Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Sat, 31 Jan 2015 17:25:39 +0000 Subject: [PATCH 05/14] Fixed malformed package.json file --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 7b89a6d..3e3444f 100644 --- a/package.json +++ b/package.json @@ -14,10 +14,10 @@ "gitup-mailer" ], "author": "Ivan Fraixedes (http://ivan.fraixed.es", - "contributors": [{ + "contributors": [{ "name" : "Gajjar Gaurav", "email" : "gajjargaurav@gmail.com" - }] + }], "license": "MIT", "bugs": { "url": "https://github.com/ifraixedes/gitup-mailer/issues" From c00d1e0b59487c60e2e0ac15f028c28a28257055 Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Tue, 3 Feb 2015 21:55:13 +0000 Subject: [PATCH 06/14] Fixed malformed package.json and added `lint` exec --- package.json | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 3e3444f..db47904 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,8 @@ "description": "a mailer module for gitup", "main": "lib/index.js", "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" + "test": "echo \"Error: no test specified\" && exit 1", + "lint": "eslint lib/*.js || true" }, "repository": { "type": "git", @@ -14,13 +15,19 @@ "gitup-mailer" ], "author": "Ivan Fraixedes (http://ivan.fraixed.es", - "contributors": [{ - "name" : "Gajjar Gaurav", - "email" : "gajjargaurav@gmail.com" - }], + "contributors": [ + { + "name": "Gajjar Gaurav", + "email": "gajjargaurav@gmail.com" + } + ], "license": "MIT", "bugs": { "url": "https://github.com/ifraixedes/gitup-mailer/issues" }, - "homepage": "https://github.com/ifraixedes/gitup-mailer" + "homepage": "https://github.com/ifraixedes/gitup-mailer", + "devDependencies": { + "eslint": "^0.13.0" + }, + "dependencies": {} } From 8f0e78cbe8a56663b99f67e36d6f394b79052c84 Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Tue, 3 Feb 2015 23:13:26 +0000 Subject: [PATCH 07/14] Added configuration for eslinter --- .eslintrc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .eslintrc diff --git a/.eslintrc b/.eslintrc new file mode 100644 index 0000000..0a2cdb3 --- /dev/null +++ b/.eslintrc @@ -0,0 +1,22 @@ +{ + "env": { + "node": true + }, + "rules": { + "quotes": [2, "single", "avoidi-scape"], + "no-use-before-define": [2, "nofunc"], + "no-multiple-empty-lines": [2, {"max": 1}], + "no-nested-ternary": 2, + "no-space-before-semi": 2, + "no-trailing-spaces": 2, + "no-irregular-whitespace": 2, + "use-isnan": 2, + "valid-jsdoc": [2, { + "prefer": { + "return": "returns" + }, + "requireReturn": false + }], + "max-len": [1, 120, 4] + } +} From bf4d8ba2925a8a912f004cdd851516b2b857a582 Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Tue, 3 Feb 2015 23:14:54 +0000 Subject: [PATCH 08/14] Implemented first version of mailer --- lib/index.js | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 lib/index.js diff --git a/lib/index.js b/lib/index.js new file mode 100644 index 0000000..a0fc704 --- /dev/null +++ b/lib/index.js @@ -0,0 +1,154 @@ +'use strict'; + +module.exports = gitEventsMailer; + +/** + * gitevents plugin API which setup an gitevents-mailer + * + * @param {Object} opts Options map to setup the mailer: + * - {gitevents-mailer/provider} provider configured email provider + * - {gitevents-mailer/renderer} renderer configured renderer + * - {Object} sendParams Map with all the values required by provider.send + * @returns {Object} configured gitevents-mailer, with + * # methods: + * - send @see return Object and `send` API private function + */ +function gitEventsMailer(opts) { + if (!opts || typeof opts !== 'object') { + throw new Error('Options must be an object'); + } + + // Opts destructuring + var provider = checkProvider(opts.provider); + var renderer = checkRenderer(opts.renderer); + var sendParams = sendParameters(opts.sendParams); + + // Exposed API + return { + send: send.bind(null, provider, renderer, sendParams) + }; +} + +/** + * Send an email using the specified provider + * + * @param {gitevents-mailer/provider} provider Configured email provider object to be used + * @param {gitevents-mailer/renderer} renderer Configured renderer object to be used + * @param {Object} params Map with all the values required by provider.send + * @param {Array} to List of email addresses. An email address is an Object with + * - {String} name + * - {String} address + * @param {Object} data Map with all the values to use by renderer to create the email content + * @param {Function} done node callback convention, no data returned when successful + * @api private + */ +function send(provider, renderer, params, to, data, done) { + renderer(data, function (err, content) { + if (err) { + return done(err); + } + + provider.send( + params.from, + to, + params.cc, + params.bcc, + content.subject, + { html: content.html, text: content.text }, + params.options, + done); + }); +} + +/** + * Basic check if object is an expected provider + * + * @param {gitevents-mailer/provider} obj Configured provider + * @returns {Object} obj parameter for assigning purpose + * @throws {Error} if provider does not fulfil the minimum expected API + * @api private + */ +function checkProvider(obj) { + if (!typeCheck(false, 'object', obj)) { + throw new Error('provider must be an object'); + } + + if (typeof obj.send !== 'function') { + throw new Error('provider must have a `send` method'); + } + + return obj; +} + +/** + * Basic check if fn is an accepted renderer + * + * @param {Function} fn configured gitevents-mailer/renderer which is a function + * @returns {Function} fn parameter for assigning + * @throws {Error} if renderer does not fulfil the minimum expected API + * @api private + */ +function checkRenderer(fn) { + if (!typeCheck(false, 'function', fn)) { + throw new Error('renderer must be a function'); + } + + return fn; +} + +/** + * Check required send parameters + * + * @param {Object} params the parameters to pass to provder.send + * @returns {Object} params parameter for assigning + * @throws {Error} if params does not contains the minimum expected values + * @api private + */ +function sendParameters(params) { + if (!typeCheck(false, 'object', params)) { + throw new Error('params must be an object'); + } + + if (!typeCheck(false, 'object', params.from)) { + throw new Error('params#from must be an object'); + } + + ['name', 'address'].forEach(function (prop) { + if (!typeCheck(false, 'string', params.from[prop])) { + throw new Error('params#from must have `name` and `address`'); + } + }); + + if (!typeCheck(true, Array, params.cc) || !typeCheck(true, Array, params.bcc)) { + throw new Error('params#cc and params#bcc must be an Array when provided'); + } + + if (!typeCheck(true, 'object', params.options)) { + throw new Error('params#options must be an Object when provided'); + } + + // Assign null to cc & bcc if they're not provided + if (!params.cc) { params.cc = null; } + if (!params.bcc) { params.bcc = null; } + + return params; +} + +/** + * Helper function to check if a value can be empty and its type + * + * @param {boolean} allowEmpty if empty values are allowed + * @param {String|Function} type to check the target, if string typeof is used otherwise instanceof + * @param {*} target th value to check + * @returns {boolean} True if target type checks, otherwise false + */ +function typeCheck(allowEmpty, type, target) { + if (allowEmpty && !target) { + return true; + } + + if (!target) { return false; } + + return (typeof type === 'string') ? + typeof target === type : target instanceof type; +} From ffb81d2e320e1e873a6ea037e93cdc9506f3d19b Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Wed, 4 Feb 2015 00:18:30 +0000 Subject: [PATCH 09/14] Provide a callback to send if it's not provided --- lib/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/index.js b/lib/index.js index a0fc704..930682a 100644 --- a/lib/index.js +++ b/lib/index.js @@ -43,6 +43,8 @@ function gitEventsMailer(opts) { * @api private */ function send(provider, renderer, params, to, data, done) { + if (!done) { done = function () {}; } + renderer(data, function (err, content) { if (err) { return done(err); From 9351eeebffcc9a5bcb229eadb41a3581987744da Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Wed, 4 Feb 2015 00:19:11 +0000 Subject: [PATCH 10/14] Updated README with the changes that first implementation has brought --- README.md | 82 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index f167a82..b18d5ab 100644 --- a/README.md +++ b/README.md @@ -18,13 +18,13 @@ To contribute to discuss the specification and APIs, go to the specific issue ## Specification -- Implement a core without lock in any specific email provider nor template library which fulfil [gitevents](https://github.com/GitEvents/gitevents) needs. +- Implement a core without lock in any specific email provider nor template library which fulfil [gitevents](https://github.com/gitevents/gitevents) needs. - Send mass emails to subscribers (e.g. newsletters). - Support different template engines, we call them renderers. -- Support different email providers, we call them mailers; each provider must transform option parameters to the expected format and passing its expected parameters. +- Support different email providers, we call them providers; each provider must transform option parameters to the expected format and passing its expected parameters. - Only offer global and generic email options; any information in the content of the email has to be set in the template with a variable, although they are recommended for example to render on any client or to reduce to get capture by the spam filter, they don't have to be in the modules, that task relies in the person who build the template, not in this module. For this reason the module will be pluggable for: - Renderers - - Mailers + - Providers - Possibility to use features offered by several providers, for example, email addresses list of the subscribers, etc. - Keep simple and modular. @@ -32,35 +32,43 @@ We've already know that this specification is challenging, due to keep it simple ## Module API -The module API must fulfil the [gitevents](https://github.com/GitEvents/gitevents) API, so far it's under definiton on [GitEvents/gitevents #45](https://github.com/GitEvents/gitevents/issues/45) +The module API must fulfil the [gitevents](https://github.com/gitevents/gitevents) API, so far it's under definiton on [GitEvents/gitevents #45](https://github.com/GitEvents/gitevents/issues/45) -The first approach that we've thought is exporting a function which receive an options object, then it returns something that we have to define if it's a function or an object. +The first approach that we've thought is exporting a function which receive an options object, then it returns and object with the methods that the plugin expose. + +The exported object is a setup mailer that allows to send emails with the base configuration, it means that the emails will always be equal with the only exception of the receivers (`to` address list) and the data to use to render the content of the email. If you have to send an email with a different configuration then, get a new one calling the plugin function with the new configuration. it has been though in this way due that how we think that [gitevents microservice](https://github.com/gitevents/gitevents) should work, that basically it should configure all the actions (plugins) in the boostrapt and use them when GitHub web hook is fired; if the configuration change, then microservice should restart, or maybe do a hot swap but not to have to configure the plugins on every web hook call that requires that specific action (plugin). + +This plugin return the object with one only method, `send` which must receive 3 arguments: + * `{Object} to`: this parameter will be passed straightaway to [provider.send](#provider-api-plugin) For this module we've thought that options object should have as required options: -* mailer: The mailer module which implement the [Mailer API plugin](#mailer-api-plugin) -* renderer: The renderer module which implement the [Renderer API plugin](#renderer-api-plugin) +* `provider`: The provider module which implement the [Provider API plugin](#provider-api-plugin) +* `renderer`: The renderer module which implement the [Renderer API plugin](#renderer-api-plugin) +* `sendParams`: An object with all the required and optional parameters that `provider.send` require and that only have to be passed ones, they are: `from`, `cc`, `bcc` and `options`, if they are optional then they don't need to be provided, see them in [Provider API plugin](#provider-api-plugin) +* `done`: Node callback function convention. + -## Mailer API plugin +## Provider API plugin -The first approach that we've thought for the mailer's API is +The first approach that we've thought for the provider's API is -Any __mailer__ plugin must export a function wich takes an `options {Object}` which contains the parameter that the email provider wrapped by the plugin needs; having an object we don't have to take car if a provider need more options or less than others. +Any __provider__ plugin must export a function wich takes an `options {Object}` which contains the parameter that the email provider wrapped by the plugin needs; having an object we don't have to take car if a provider needs more options or less than others. And it __returns an object__ which at least must have `send` method; it could be just a function, but an object has more flexibility to offer more methods if we need in the future, making easy backward compatibility. For `send` method we've roughly thought to have the next fixed parameters: -1. `from {Object}`: Object with 2 properties: - * `name {String}`: Name to show. - * `address {String}`: email address. -2. `to {Array}`: Array of objects with the same 2 properties than `from` object. -3. `cc {Array}`: Same as `to` but for `cc`. -4. `bcc {Array}`: Same as `to` but for `bcc`. -5. `subject {String}`: Message subject. -6. `body {Object}`: Object with 2 properties: - * `html {String}`: HTML version of the message's body. - * `text {String}`: Plain text version of the message's body. -7. `options {Object}` (optional): An object with specific properties of the implemented provider. Note they are unlikely compatible between different providers, therefore if you change the mailer, then you will have to update it, moreover some mailers can ignore it. In the most of the cases, they would unlikely needed, e.g. attachments. +1. `{Object} from`: Object with 2 properties: + * `{String} name`: Name to show. + * `{String} address`: email address. +2. `{Array} to`: Array of objects with the same 2 properties than `from` object. +3. `{Array} cc`: Same as `to` but for `cc`. +4. `{Array} bcc`: Same as `to` but for `bcc`. +5. `{String} subject`: Message subject. +6. `{Object} body`: Object with 2 properties: + * `{String} html`: HTML version of the message's body. + * `{String} text`: Plain text version of the message's body. +7. `{Object} options` (optional): An object with specific properties of the implemented provider. Note they are unlikely compatible between different providers, therefore if you change the provider, then you will have to update it, moreover some providers can ignore it. In the most of the cases, they would unlikely needed, e.g. attachments. Send method may contain some parameters that they aren't often used, however we thought to have as a parameter because they are very common to any email send action; however to avoid the complexity to manage optional parameters moreover they make the code more difficult to understand, if there are too many, then we may move them to the `options` object. @@ -68,27 +76,33 @@ Send method may contain some parameters that they aren't often used, however we Any __renderer__ plugin module must export a function which must receive the next parameters: -1. `htmlTemplate {String}`: A string with the HTML version of the template to render. -2. `textTemplate {String}`: A string with the plain text version of the template to render. -3. `locals {Object}`: An object which contains the template variables values to use as default, if none are needed then `null`. -4. `config {Object} (optional)`: An object which contains specific configurations for the template library. Note they are unlikely compatible between different renderers, hence if you change the renderer then you may have to update it, furthermore some renderer may ignore this parameters. +1. `{String} subject`: A string with the text template to use as subject. +2. `{String} htmlTemplate`: A string with the HTML version of the template to render*1. +3. `{String} textTemplate`: A string with the plain text version of the template to render*1. +4. `{Object} locals`: An object which contains the template variables values to use as default, if none are needed then `null`. +5. `{Object} config (optional)`: An object which contains specific configurations for the template library. Note they are unlikely compatible between different renderers, hence if you change the renderer then you may have to update it, furthermore some renderer may ignore this parameters. Although __two templates types have to pass as a parameter we may only require one__, then one of them can be null, obviously the result will only contain the provided one. + *1 it can be the string with the template content, a file path, an URL or whatever the renderer supports. And it has to __return__ a `function` which only accepts one parameter: - 1. `data {Object}`: An object which contains the template variable values to use; if one of them is not provided and it has been defined in `locals` then its values will be used. + 1. `data {Object}`: An object which contains the template variable values to use; if one of them is not provided and it has been defined in `locals` then its values will be used. If no data, then `null`. + 2. `done {Function}`: Node callback function convention, which if succeeds return an object with the next properties: + * `{String} subject`: The subject rendered text. + * `{String} html`: The rendered HTML version of the template or null if HTML version hasn't been provided. + * `{String} text`: The rendered plain text version of the template or null if plain text version hasn't been provided. - This function must __return__ an Object with two properties: - * `html {String}`: The rendered HTML version of the template or null if HTML version hasn't been provided. - * `text {String}`: The rendered plain text version of the template or null if plain text version hasn't been provided. +~~We know that the most of the template engines render templates from files, however to keep the API simple we suggest to only render string templates for two main reasons:~~ -We know that the most of the template engines render templates from files, however to keep the API simple we suggest to only render string templates for two main reasons: +In the first draft of this document, before any implementation the previous strikethrough lines where mentioned, for the next two reasons: -1. Render from file is something we want to do, however reading a file from a disk is something that can be done as standalone tool/util/helper and avoid complexity in renderers implementations and duplicate functionality which should be tested as well. I've also thought that email templates are quite complicated and they have a lot of pain points if they are compatible for at least the most email clients, desktop and browser ones, then keeping them as a single template without partials and layout extension (inheritance), so a single and simple string is enough. -2. Even though to render templates from files seem as a requirement for `gitevents`, we consider that it isn't because the group want to host the template in a public URL (e.g. a Github gist), then the renderer's API should have another function for templates available in an URL, then as I've commented previously the API should be more complex and probably the renders would have duplicated functionality. +>1. Render from file is something we want to do, however reading a file from a disk is something that can be done as standalone tool/util/helper and avoid complexity in renderers implementations and duplicate functionality which should be tested as well. I've also thought that email templates are quite complicated and they have a lot of pain points if they are compatible for at least the most email clients, desktop and browser ones, then keeping them as a single template without partials and layout extension (inheritance), so a single and simple string is enough. +>2. Even though to render templates from files seem as a requirement for `gitevents`, we consider that it isn't because the group want to host the template in a public URL (e.g. a Github gist), then the renderer's API should have another function for templates available in an URL, then as I've commented previously the API should be more complex and probably the renders would have duplicated functionality. +> +>A challenging point here is if with this interface we could implement a renderer which use remote templates hosted by the email provider, for example, using a mailchimp provider we can implement a specific renderer that allows to the provider to use its template API, of course mailchimp renderer would only work with mailchimp provider, however mailchimp provider must work as any other renderer which is not specific of an email provider. -A challenging point here is if with this interface we could implement a renderer which use remote templates hosted by the email provider, for example, using a mailchimp mailer we can implement a specific renderer that allows to the emailer to use its template API, of course mailchimp renderer would only work with mailchimp mailer, however mailchimp mailer must work as any other renderer which is not specific of an email provider. +Those reasons still partially applicable, however, if the plugin has to be setup in the bootstrap of [gitevents microservice](https://github.com/gitevents/gitevents) and use it to send a type of email per instance and avoid to implement the logic of rendering the email content and passing it to each `send`, it has been moved inside this plugin, which makes sense because they're bound and have the same target; for this reason the renderer receive a callback because it may be asynchronous if the templates are files paths, URL or another kind of IO. Nonetheless we could be implemented in somewhere that can be reused, see [utilities section](#Utilities) So far we have an example of a renderer, however it was implemented with the first hackday which we ran on 18/01/2015 and it was a pair programming with a teaching purpose; this document was not available at that moment, for that reason it does not fulfil with the current Renderer API plugin. @@ -105,4 +119,4 @@ We leave this section to think about to have other kind of plugins to perform re ## LICENSE -MIT license, read [LICENSE file](https://raw.githubusercontent.com/ifraixedes/gitevents-mailer/master/LICENSE) for more information. +MIT license, read [LICENSE file](https://github.com/GitEvents/gitevents-mailer/blob/master/LICENSE) for more information. From d65959dc81d00e71baa73d11a5d8163b776ad441 Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Thu, 12 Feb 2015 01:18:44 +0000 Subject: [PATCH 11/14] clearer some error messages --- lib/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/index.js b/lib/index.js index 930682a..28f8d12 100644 --- a/lib/index.js +++ b/lib/index.js @@ -72,7 +72,7 @@ function send(provider, renderer, params, to, data, done) { */ function checkProvider(obj) { if (!typeCheck(false, 'object', obj)) { - throw new Error('provider must be an object'); + throw new Error('provider is required and must be an object'); } if (typeof obj.send !== 'function') { @@ -92,7 +92,7 @@ function checkProvider(obj) { */ function checkRenderer(fn) { if (!typeCheck(false, 'function', fn)) { - throw new Error('renderer must be a function'); + throw new Error('renderer is required and must be a function'); } return fn; @@ -108,7 +108,7 @@ function checkRenderer(fn) { */ function sendParameters(params) { if (!typeCheck(false, 'object', params)) { - throw new Error('params must be an object'); + throw new Error('params is required and must be an object'); } if (!typeCheck(false, 'object', params.from)) { From 750e10045ad56e14ef86323bf0e91fce46a17976 Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Thu, 12 Feb 2015 01:19:21 +0000 Subject: [PATCH 12/14] Created unit test --- package.json | 7 +-- test/.eslintrc | 17 +++++++ test/mailer_test.js | 110 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 test/.eslintrc create mode 100644 test/mailer_test.js diff --git a/package.json b/package.json index db47904..72f3c55 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,8 @@ "description": "a mailer module for gitup", "main": "lib/index.js", "scripts": { - "test": "echo \"Error: no test specified\" && exit 1", - "lint": "eslint lib/*.js || true" + "test": "node test/*_test.js", + "lint": "eslint lib/*.js test/*.js || true" }, "repository": { "type": "git", @@ -27,7 +27,8 @@ }, "homepage": "https://github.com/ifraixedes/gitup-mailer", "devDependencies": { - "eslint": "^0.13.0" + "eslint": "^0.13.0", + "tape": "^3.5.0" }, "dependencies": {} } diff --git a/test/.eslintrc b/test/.eslintrc new file mode 100644 index 0000000..7a8b7de --- /dev/null +++ b/test/.eslintrc @@ -0,0 +1,17 @@ +{ + "env": { + "node": true + }, + "rules": { + "quotes": [2, "single", "avoidi-scape"], + "no-use-before-define": [2, "nofunc"], + "no-multiple-empty-lines": [2, {"max": 1}], + "no-nested-ternary": 2, + "no-space-before-semi": 2, + "no-trailing-spaces": 2, + "no-irregular-whitespace": 2, + "use-isnan": 2, + "max-len": [1, 120, 4], + "no-shadow": false + } +} diff --git a/test/mailer_test.js b/test/mailer_test.js new file mode 100644 index 0000000..09cc516 --- /dev/null +++ b/test/mailer_test.js @@ -0,0 +1,110 @@ +'use strict'; + +var test = require('tape'); +var mailerPlugin = require('../lib'); + +function fakeRenderer(data, done) { + setImmediate(done.bind(null), null, { + subject: 'fake subject', + html: 'fake html body', + text: 'fake plain text body' + }); +} + +var fakeProvider = { + send: function (from, to, cc, bcc, subject, bodies, opts, done) { + setImmediate(done.bind(null), null, { + from: from, + to: to, + cc: cc, + bcc: bcc, + subject: subject, + bodies: bodies, + options: opts + }); + } +}; + +var sendParams = { + from: { + name: 'Ivan', + address: 'ivan@te.st' + } +}; + +test('GitEvents-Mailer', function (t) { + + test('plugin', function (t) { + t.equal(typeof mailerPlugin, 'function', 'exports a function'); + t.throws(mailerPlugin.bind(null, 'options'), /options must be an object/i, + 'exported function takes one options parameter which must be an object'); + t.throws(mailerPlugin.bind(null, { some: 'options' }), /required/i, + 'exported function options parameter requires provider, renderer and sendParams'); + t.throws(mailerPlugin.bind(null, { renderer: fakeRenderer }), /required/i, + 'exported function options parameter requires provider, sendParams as well'); + t.throws(mailerPlugin.bind(null, { renderer: fakeRenderer, provider: fakeProvider }), /required/i, + 'exported function options parameter requires sendParams as well'); + t.throws(mailerPlugin.bind(null, { + renderer: {}, + provider: fakeProvider, + sendParams: sendParams }), + /must be a function/i, + 'exported function options parameter requires renderer to be a function'); + + t.throws(mailerPlugin.bind(null, { + renderer: { }, + provider: fakeProvider, + sendParams: sendParams }), + /must be a function/i, 'exported function options parameter requires renderer to be a function'); + + t.throws(mailerPlugin.bind(null, { + renderer: fakeRenderer, + provider: { received: function () { } }, + sendParams: sendParams + }), /.*send.* method/i, 'exported function options parameter requires provider to have send method'); + + t.throws(mailerPlugin.bind(null, { + renderer: fakeRenderer, + provider: fakeProvider, + sendParams: { to: 'some' } + }), /from must be an Object/i, 'exported function options parameter requires sendParmas to have `from` property'); + + t.throws(mailerPlugin.bind(null, { + renderer: fakeRenderer, + provider: fakeProvider, + sendParams: { from: { address: 'ivan@te.st' } } + }), /from must have .*name.*/i, + 'exported function options parameter requires sendParams#from to have `name` property'); + + t.throws(mailerPlugin.bind(null, { + renderer: fakeRenderer, + provider: fakeProvider, + sendParams: { from: { name: 'ivan' } } + }), /from must have .*address.*/i, + 'exported function options parameter requires sendParams#from to have `address` property'); + + t.doesNotThrow(mailerPlugin.bind(null, { renderer: fakeRenderer, provider: fakeProvider, sendParams: sendParams }), + 'exported function returns a mailer object when options contains all the expected parameters'); + t.end(); + }); + + test('mailer object', function (t) { + var mailer = mailerPlugin({ renderer: fakeRenderer, provider: fakeProvider, sendParams: sendParams }); + var to = [{ name: 'Bill', address: 'bill@te.st' }]; + + t.plan(9); + t.equals(typeof mailer.send, 'function', 'has a `send` method'); + mailer.send(to, null, function (err, allParams) { + t.equals(err, null, 'send does not return any error'); + t.equals(allParams.from, sendParams.from); + t.equals(allParams.to, to); + t.equals(allParams.cc, sendParams.cc); + t.equals(allParams.bcc, sendParams.bcc); + t.equals(allParams.subject, 'fake subject'); + t.deepEqual(allParams.bodies, { html: 'fake html body', text: 'fake plain text body' }); + t.equals(allParams.options, sendParams.options); + }); + }); + + t.end(); +}); From d3e5c63fed997fcc74dfef40d59692ed744e03fb Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Wed, 18 Feb 2015 00:38:44 +0000 Subject: [PATCH 13/14] Fixed missed data and argument descpription placed wrongly --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b18d5ab..2af76cc 100644 --- a/README.md +++ b/README.md @@ -40,12 +40,13 @@ The exported object is a setup mailer that allows to send emails with the base c This plugin return the object with one only method, `send` which must receive 3 arguments: * `{Object} to`: this parameter will be passed straightaway to [provider.send](#provider-api-plugin) + * `{Object} data`: this parameter will be passed straightaway to an instance of [renderer](#renderer-api-plugin) (the function returned) + * `done`: Node callback function convention. For this module we've thought that options object should have as required options: * `provider`: The provider module which implement the [Provider API plugin](#provider-api-plugin) * `renderer`: The renderer module which implement the [Renderer API plugin](#renderer-api-plugin) * `sendParams`: An object with all the required and optional parameters that `provider.send` require and that only have to be passed ones, they are: `from`, `cc`, `bcc` and `options`, if they are optional then they don't need to be provided, see them in [Provider API plugin](#provider-api-plugin) -* `done`: Node callback function convention. ## Provider API plugin From eb8b6f6775815147a8b003e0d8baf4a35396bf1b Mon Sep 17 00:00:00 2001 From: Ivan Fraixedes Date: Tue, 17 Mar 2015 22:55:29 +0000 Subject: [PATCH 14/14] Fixed description numb of params renderer returned function in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2af76cc..30c5375 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ Any __renderer__ plugin module must export a function which must receive the nex Although __two templates types have to pass as a parameter we may only require one__, then one of them can be null, obviously the result will only contain the provided one. *1 it can be the string with the template content, a file path, an URL or whatever the renderer supports. - And it has to __return__ a `function` which only accepts one parameter: + And it has to __return__ a `function` which accepts two parameter: 1. `data {Object}`: An object which contains the template variable values to use; if one of them is not provided and it has been defined in `locals` then its values will be used. If no data, then `null`. 2. `done {Function}`: Node callback function convention, which if succeeds return an object with the next properties: