From 819fcfb3929292cf894aa35a88ec0a7b4dff0726 Mon Sep 17 00:00:00 2001 From: Krzysztof Jablonski Date: Thu, 8 Jun 2017 14:06:35 +0200 Subject: [PATCH 01/12] Fix typo during refactoring During code refactoring for selectable unsubscription feature code: `!campaignId || status > 2` was wrongly refactored to: `subscription.status !== Status.SUBSCRIBED` Link: https://github.com/Mailtrain-org/mailtrain/commit/a6d25e668b6d9a31b90b05bb9bfe7d65f1d619a1#diff-5af9fe5dfae76c093530c92e3d7404e1R496 --- lib/models/subscriptions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/subscriptions.js b/lib/models/subscriptions.js index 5b2ab460..1ea574c6 100644 --- a/lib/models/subscriptions.js +++ b/lib/models/subscriptions.js @@ -495,7 +495,7 @@ module.exports.changeStatus = (listId, id, campaignId, status, callback) => { } // status change is not related to a campaign or it marks message as bounced etc. - if (!campaignId || status !== Status.SUBSCRIBED) { + if (!campaignId || status > Status.UNSUBSCRIBED) { return connection.commit(err => { if (err) { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); From 71ac4c64a5ec9c5b934dd6c7b4f2ba83833756fc Mon Sep 17 00:00:00 2001 From: Krzysztof Jablonski Date: Thu, 8 Jun 2017 14:22:34 +0200 Subject: [PATCH 02/12] =?UTF-8?q?Avoid=20using=20>=20with=20=E2=80=9Eenums?= =?UTF-8?q?=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/models/subscriptions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/subscriptions.js b/lib/models/subscriptions.js index 1ea574c6..6760bda9 100644 --- a/lib/models/subscriptions.js +++ b/lib/models/subscriptions.js @@ -495,7 +495,7 @@ module.exports.changeStatus = (listId, id, campaignId, status, callback) => { } // status change is not related to a campaign or it marks message as bounced etc. - if (!campaignId || status > Status.UNSUBSCRIBED) { + if (!campaignId || status !== Status.SUBSCRIBED && status !== Status.UNSUBSCRIBED) { return connection.commit(err => { if (err) { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); From 87f7a050ab5882f71b4dd535777b5fed19286bfe Mon Sep 17 00:00:00 2001 From: witzig Date: Sun, 11 Jun 2017 01:26:15 +0200 Subject: [PATCH 03/12] Fixed bug #249 --- routes/api.js | 7 ++++++- routes/subscription.js | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/routes/api.js b/routes/api.js index a44d6fec..a12e6b38 100644 --- a/routes/api.js +++ b/routes/api.js @@ -125,7 +125,12 @@ router.post('/subscribe/:listId', (req, res) => { } if (/^(yes|true|1)$/i.test(input.REQUIRE_CONFIRMATION)) { - confirmations.addConfirmation(list.id, 'subscribe', req.ip, subscription, (err, confirmCid) => { + const data = { + email: subscription.email, + subscriptionData: subscription + }; + + confirmations.addConfirmation(list.id, 'subscribe', req.ip, data, (err, confirmCid) => { if (err) { log.error('API', err); res.status(500); diff --git a/routes/subscription.js b/routes/subscription.js index 6310bbf3..2693deda 100644 --- a/routes/subscription.js +++ b/routes/subscription.js @@ -77,6 +77,7 @@ router.get('/confirm/subscribe/:cid', (req, res, next) => { let optInCountry = geoip.lookupCountry(confirmation.ip) || null; const meta = { + cid: req.params.cid, email: data.email, optInIp: confirmation.ip, optInCountry, @@ -438,7 +439,7 @@ router.post('/:cid/subscribe', passport.parseForm, corsOrCsrfProtection, (req, r log.info('Subscription', 'Confirmation message for %s marked to be skipped (%s)', email, JSON.stringify(data)); sendWebResponse(); } else { - mailHelpers.sendConfirmSubscription(list, email, confirmCid, data, (err) => { + mailHelpers.sendConfirmSubscription(list, email, confirmCid, subscriptionData, (err) => { if (err) { return req.xhr ? sendJsonError(err) : sendWebResponse(err); } @@ -789,7 +790,7 @@ function handleUnsubscribe(list, subscription, autoUnsubscribe, campaignId, ip, if (err) { return next(err); } - + // TODO: Shall we do anything with "found"? mailHelpers.sendUnsubscriptionConfirmed(list, subscription.email, subscription, err => { From 0629194f569ddf93b52f5a0f4f3aa21888f5cf2a Mon Sep 17 00:00:00 2001 From: witzig Date: Sun, 11 Jun 2017 18:48:41 +0200 Subject: [PATCH 04/12] Updated e2e subscription tests Added test for bug #249 Extended tests for custom fields (work in progress) --- package.json | 1 + setup/sql/mailtrain-test.sql | 14 +- test/e2e/lib/config.js | 24 ++- test/e2e/lib/page.js | 4 +- test/e2e/page-objects/subscription.js | 47 ++++- test/e2e/tests/subscription.js | 288 +++++++++++++++++++------- 6 files changed, 290 insertions(+), 88 deletions(-) diff --git a/package.json b/package.json index 926c9a2b..e466e96b 100644 --- a/package.json +++ b/package.json @@ -109,6 +109,7 @@ "redfour": "^1.0.0", "redis": "^2.7.1", "request": "^2.81.0", + "request-promise": "^4.2.1", "serve-favicon": "^2.4.2", "shortid": "^2.2.8", "slugify": "^1.1.0", diff --git a/setup/sql/mailtrain-test.sql b/setup/sql/mailtrain-test.sql index 95af27a0..b88e5bef 100644 --- a/setup/sql/mailtrain-test.sql +++ b/setup/sql/mailtrain-test.sql @@ -119,7 +119,7 @@ CREATE TABLE `campaigns` ( KEY `parent_index` (`parent`), KEY `check_index` (`last_check`) ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8mb4; -INSERT INTO `campaigns` (`id`, `cid`, `type`, `parent`, `name`, `description`, `list`, `segment`, `template`, `source_url`, `editor_name`, `editor_data`, `last_check`, `check_status`, `from`, `address`, `reply_to`, `subject`, `html`, `html_prepared`, `text`, `status`, `scheduled`, `status_change`, `delivered`, `blacklisted`, `opened`, `clicks`, `unsubscribed`, `bounced`, `complained`, `created`, `open_tracking_disabled`, `click_tracking_disabled`) VALUES (1,'BkwHWgCWb',1,NULL,'Merge Tags','',1,0,0,'','codeeditor',NULL,NULL,NULL,'My Awesome Company','admin@example.com','','Test message','\r\n
\r\n
LINK_UNSUBSCRIBE
\r\n \r\n
LINK_PREFERENCES
\r\n \r\n
LINK_BROWSER
\r\n \r\n
EMAIL
\r\n
[EMAIL]
\r\n
FIRST_NAME
\r\n
[FIRST_NAME]
\r\n
LAST_NAME
\r\n
[LAST_NAME]
\r\n
FULL_NAME
\r\n
[FULL_NAME]
\r\n
SUBSCRIPTION_ID
\r\n
[SUBSCRIPTION_ID]
\r\n
LIST_ID
\r\n
[LIST_ID]
\r\n
CAMPAIGN_ID
\r\n
[CAMPAIGN_ID]
\r\n
MERGE_TEXT
\r\n
[MERGE_TEXT]
\r\n
MERGE_NUMBER
\r\n
[MERGE_NUMBER]
\r\n
MERGE_WEBSITE
\r\n
[MERGE_WEBSITE]
\r\n
MERGE_GPG_PUBLIC_KEY
\r\n
[MERGE_GPG_PUBLIC_KEY/GPG Fallback Text]
\r\n
MERGE_MULTILINE_TEXT
\r\n
[MERGE_MULTILINE_TEXT]
\r\n
MERGE_JSON
\r\n
[MERGE_JSON]
\r\n
MERGE_DATE_MMDDYY
\r\n
[MERGE_DATE_MMDDYY]
\r\n
MERGE_DATE_DDMMYY
\r\n
[MERGE_DATE_DDMMYY]
\r\n
MERGE_BIRTHDAY_MMDD
\r\n
[MERGE_BIRTHDAY_MMDD]
\r\n
MERGE_BIRTHDAY_DDMM
\r\n
[MERGE_BIRTHDAY_DDMM]
\r\n
MERGE_DROP_DOWNS
\r\n
[MERGE_DROP_DOWNS]
\r\n
MERGE_CHECKBOXES
\r\n
[MERGE_CHECKBOXES]
\r\n
','\n
\n
LINK_UNSUBSCRIBE
\n \n
LINK_PREFERENCES
\n \n
LINK_BROWSER
\n \n
EMAIL
\n
[EMAIL]
\n
FIRST_NAME
\n
[FIRST_NAME]
\n
LAST_NAME
\n
[LAST_NAME]
\n
FULL_NAME
\n
[FULL_NAME]
\n
SUBSCRIPTION_ID
\n
[SUBSCRIPTION_ID]
\n
LIST_ID
\n
[LIST_ID]
\n
CAMPAIGN_ID
\n
[CAMPAIGN_ID]
\n
MERGE_TEXT
\n
[MERGE_TEXT]
\n
MERGE_NUMBER
\n
[MERGE_NUMBER]
\n
MERGE_WEBSITE
\n
[MERGE_WEBSITE]
\n
MERGE_GPG_PUBLIC_KEY
\n
[MERGE_GPG_PUBLIC_KEY/GPG Fallback Text]
\n
MERGE_MULTILINE_TEXT
\n
[MERGE_MULTILINE_TEXT]
\n
MERGE_JSON
\n
[MERGE_JSON]
\n
MERGE_DATE_MMDDYY
\n
[MERGE_DATE_MMDDYY]
\n
MERGE_DATE_DDMMYY
\n
[MERGE_DATE_DDMMYY]
\n
MERGE_BIRTHDAY_MMDD
\n
[MERGE_BIRTHDAY_MMDD]
\n
MERGE_BIRTHDAY_DDMM
\n
[MERGE_BIRTHDAY_DDMM]
\n
MERGE_DROP_DOWNS
\n
[MERGE_DROP_DOWNS]
\n
MERGE_CHECKBOXES
\n
[MERGE_CHECKBOXES]
\n
','',1,NOW(),NULL,0,0,0,0,0,0,0,NOW(),0,0); +INSERT INTO `campaigns` (`id`, `cid`, `type`, `parent`, `name`, `description`, `list`, `segment`, `template`, `source_url`, `editor_name`, `editor_data`, `last_check`, `check_status`, `from`, `address`, `reply_to`, `subject`, `html`, `html_prepared`, `text`, `status`, `scheduled`, `status_change`, `delivered`, `blacklisted`, `opened`, `clicks`, `unsubscribed`, `bounced`, `complained`, `created`, `open_tracking_disabled`, `click_tracking_disabled`) VALUES (1,'BkwHWgCWb',1,NULL,'Merge Tags','',1,0,0,'','codeeditor',NULL,NULL,NULL,'My Awesome Company','admin@example.com','','Test message','\r\n
\r\n
LINK_UNSUBSCRIBE
\r\n \r\n
LINK_PREFERENCES
\r\n \r\n
LINK_BROWSER
\r\n \r\n
EMAIL
\r\n
[EMAIL]
\r\n
FIRST_NAME
\r\n
[FIRST_NAME]
\r\n
LAST_NAME
\r\n
[LAST_NAME]
\r\n
FULL_NAME
\r\n
[FULL_NAME]
\r\n
SUBSCRIPTION_ID
\r\n
[SUBSCRIPTION_ID]
\r\n
LIST_ID
\r\n
[LIST_ID]
\r\n
CAMPAIGN_ID
\r\n
[CAMPAIGN_ID]
\r\n
MERGE_TEXT
\r\n
[MERGE_TEXT]
\r\n
MERGE_NUMBER
\r\n
[MERGE_NUMBER]
\r\n
MERGE_WEBSITE
\r\n
[MERGE_WEBSITE]
\r\n
MERGE_GPG_PUBLIC_KEY
\r\n
[MERGE_GPG_PUBLIC_KEY/GPG Fallback Text]
\r\n
MERGE_MULTILINE_TEXT
\r\n
[MERGE_MULTILINE_TEXT]
\r\n
MERGE_JSON
\r\n
[MERGE_JSON]
\r\n
MERGE_DATE_MMDDYYYY
\r\n
[MERGE_DATE_MMDDYYYY]
\r\n
MERGE_DATE_DDMMYYYY
\r\n
[MERGE_DATE_DDMMYYYY]
\r\n
MERGE_BIRTHDAY_MMDD
\r\n
[MERGE_BIRTHDAY_MMDD]
\r\n
MERGE_BIRTHDAY_DDMM
\r\n
[MERGE_BIRTHDAY_DDMM]
\r\n
MERGE_DROP_DOWNS
\r\n
[MERGE_DROP_DOWNS]
\r\n
MERGE_CHECKBOXES
\r\n
[MERGE_CHECKBOXES]
\r\n
','\n
\n
LINK_UNSUBSCRIBE
\n \n
LINK_PREFERENCES
\n \n
LINK_BROWSER
\n \n
EMAIL
\n
[EMAIL]
\n
FIRST_NAME
\n
[FIRST_NAME]
\n
LAST_NAME
\n
[LAST_NAME]
\n
FULL_NAME
\n
[FULL_NAME]
\n
SUBSCRIPTION_ID
\n
[SUBSCRIPTION_ID]
\n
LIST_ID
\n
[LIST_ID]
\n
CAMPAIGN_ID
\n
[CAMPAIGN_ID]
\n
MERGE_TEXT
\n
[MERGE_TEXT]
\n
MERGE_NUMBER
\n
[MERGE_NUMBER]
\n
MERGE_WEBSITE
\n
[MERGE_WEBSITE]
\n
MERGE_GPG_PUBLIC_KEY
\n
[MERGE_GPG_PUBLIC_KEY/GPG Fallback Text]
\n
MERGE_MULTILINE_TEXT
\n
[MERGE_MULTILINE_TEXT]
\n
MERGE_JSON
\n
[MERGE_JSON]
\n
MERGE_DATE_MMDDYYYY
\n
[MERGE_DATE_MMDDYYYY]
\n
MERGE_DATE_DDMMYYYY
\n
[MERGE_DATE_DDMMYYYY]
\n
MERGE_BIRTHDAY_MMDD
\n
[MERGE_BIRTHDAY_MMDD]
\n
MERGE_BIRTHDAY_DDMM
\n
[MERGE_BIRTHDAY_DDMM]
\n
MERGE_DROP_DOWNS
\n
[MERGE_DROP_DOWNS]
\n
MERGE_CHECKBOXES
\n
[MERGE_CHECKBOXES]
\n
','',1,NOW(),NULL,0,0,0,0,0,0,0,NOW(),0,0); CREATE TABLE `confirmations` ( `id` int(11) unsigned NOT NULL AUTO_INCREMENT, `cid` varchar(255) CHARACTER SET ascii NOT NULL, @@ -156,8 +156,8 @@ INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (4,1,'GPG Public Key','MERGE_GPG_PUBLIC_KEY',NULL,'gpg',NULL,NULL,'custom_gpg_public_key_ryvj51cz',1,NOW()); INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (5,1,'Multiline Text','MERGE_MULTILINE_TEXT',NULL,'longtext',NULL,NULL,'custom_multiline_text_bjbfojawb',1,NOW()); INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (6,1,'JSON','MERGE_JSON',NULL,'json',NULL,NULL,'custom_json_skqjkcb',1,NOW()); -INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (7,1,'Date (MM/DD/YY)','MERGE_DATE_MMDDYY',NULL,'date-us',NULL,NULL,'custom_date_mmddyy_rjkeojrzz',1,NOW()); -INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (8,1,'Date (DD/MM/YY)','MERGE_DATE_DDMMYY',NULL,'date-eur',NULL,NULL,'custom_date_ddmmyy_ryedsk0wz',1,NOW()); +INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (7,1,'Date (MM/DD/YYYY)','MERGE_DATE_MMDDYYYY',NULL,'date-us',NULL,NULL,'custom_date_mmddyy_rjkeojrzz',1,NOW()); +INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (8,1,'Date (DD/MM/YYYY)','MERGE_DATE_DDMMYYYY',NULL,'date-eur',NULL,NULL,'custom_date_ddmmyy_ryedsk0wz',1,NOW()); INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (9,1,'Birthday (MM/DD)','MERGE_BIRTHDAY_MMDD',NULL,'birthday-us',NULL,NULL,'custom_birthday_mmdd_h18coj0zz',1,NOW()); INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (10,1,'Birthday (DD/MM)','MERGE_BIRTHDAY_DDMM',NULL,'birthday-eur',NULL,NULL,'custom_birthday_ddmm_r1g3s1czz',1,NOW()); INSERT INTO `custom_fields` (`id`, `list`, `name`, `key`, `default_value`, `type`, `group`, `group_template`, `column`, `visible`, `created`) VALUES (11,1,'Drop Downs','MERGE_DROP_DOWNS',NULL,'dropdown',NULL,NULL,NULL,1,NOW()); @@ -324,21 +324,21 @@ CREATE TABLE `settings` ( `value` text NOT NULL, PRIMARY KEY (`id`), UNIQUE KEY `key` (`key`) -) ENGINE=InnoDB AUTO_INCREMENT=114 DEFAULT CHARSET=utf8mb4; +) ENGINE=InnoDB AUTO_INCREMENT=148 DEFAULT CHARSET=utf8mb4; INSERT INTO `settings` (`id`, `key`, `value`) VALUES (1,'smtp_hostname','localhost'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (2,'smtp_port','5587'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (3,'smtp_encryption','NONE'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (4,'smtp_user','testuser'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (5,'smtp_pass','testpass'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (6,'service_url','http://localhost:3000/'); -INSERT INTO `settings` (`id`, `key`, `value`) VALUES (7,'admin_email','admin@example.com'); +INSERT INTO `settings` (`id`, `key`, `value`) VALUES (7,'admin_email','keep.admin@mailtrain.org'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (8,'smtp_max_connections','5'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (9,'smtp_max_messages','100'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (10,'smtp_log',''); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (11,'default_sender','My Awesome Company'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (12,'default_postaddress','1234 Main Street'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (13,'default_from','My Awesome Company'); -INSERT INTO `settings` (`id`, `key`, `value`) VALUES (14,'default_address','admin@example.com'); +INSERT INTO `settings` (`id`, `key`, `value`) VALUES (14,'default_address','keep.admin@mailtrain.org'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (15,'default_subject','Test message'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (16,'default_homepage','https://mailtrain.org'); INSERT INTO `settings` (`id`, `key`, `value`) VALUES (17,'db_schema_version','29'); @@ -1229,7 +1229,7 @@ CREATE TABLE `users` ( KEY `check_reset` (`username`(191),`reset_token`,`reset_expire`), KEY `token_index` (`access_token`) ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8mb4; -INSERT INTO `users` (`id`, `username`, `password`, `email`, `access_token`, `reset_token`, `reset_expire`, `created`) VALUES (1,'admin','$2a$10$mzKU71G62evnGB2PvQA4k..Wf9jASk.c7a8zRMHh6qQVjYJ2r/g/K','admin@example.com',NULL,NULL,NULL,NOW()); +INSERT INTO `users` (`id`, `username`, `password`, `email`, `access_token`, `reset_token`, `reset_expire`, `created`) VALUES (1,'admin','$2a$10$mzKU71G62evnGB2PvQA4k..Wf9jASk.c7a8zRMHh6qQVjYJ2r/g/K','keep.admin@mailtrain.org','7833d148e22c85474c314f43ae4591a7c9adec26',NULL,NULL,NOW()); SET UNIQUE_CHECKS=1; SET FOREIGN_KEY_CHECKS=1; diff --git a/test/e2e/lib/config.js b/test/e2e/lib/config.js index 223d2103..2fa99a3e 100644 --- a/test/e2e/lib/config.js +++ b/test/e2e/lib/config.js @@ -9,7 +9,9 @@ module.exports = { users: { admin: { username: 'admin', - password: 'test' + password: 'test', + email: 'keep.admin@mailtrain.org', + accessToken: '7833d148e22c85474c314f43ae4591a7c9adec26' } }, lists: { @@ -18,41 +20,59 @@ module.exports = { cid: 'Hkj1vCoJb', publicSubscribe: 1, unsubscriptionMode: 0, // (one-step, no form) + customFields: [ + { type: 'text', key: 'MERGE_TEXT', column: 'custom_text_field_byiiqjrw' }, + { type: 'number', key: 'MERGE_NUMBER', column: 'custom_number_field_r1dd91awb' }, + { type: 'website', key: 'MERGE_WEBSITE', column: 'custom_website_field_rkq991cw' }, + { type: 'gpg', key: 'MERGE_GPG_PUBLIC_KEY', column: 'custom_gpg_public_key_ryvj51cz' }, + { type: 'longtext', key: 'MERGE_MULTILINE_TEXT', column: 'custom_multiline_text_bjbfojawb' }, + { type: 'json', key: 'MERGE_JSON', column: 'custom_json_skqjkcb' }, + { type: 'date-us', key: 'MERGE_DATE_MMDDYYYY', column: 'custom_date_mmddyy_rjkeojrzz' }, + { type: 'date-eur', key: 'MERGE_DATE_DDMMYYYY', column: 'custom_date_ddmmyy_ryedsk0wz' }, + { type: 'birthday-us', key: 'MERGE_BIRTHDAY_MMDD', column: 'custom_birthday_mmdd_h18coj0zz' }, + { type: 'birthday-eur', key: 'MERGE_BIRTHDAY_DDMM', column: 'custom_birthday_ddmm_r1g3s1czz' }, + // TODO: Add remaining custom fields, dropdowns and checkboxes + ] }, l2: { id: 2, cid: 'SktV4HDZ-', publicSubscribe: 1, unsubscriptionMode: 1, // (one-step, with form) + customFields: [] }, l3: { id: 3, cid: 'BkdvNBw-W', publicSubscribe: 1, unsubscriptionMode: 2, // (two-step, no form) + customFields: [] }, l4: { id: 4, cid: 'rJMKVrDZ-', publicSubscribe: 1, unsubscriptionMode: 3, // (two-step, with form) + customFields: [] }, l5: { id: 5, cid: 'SJgoNSw-W', publicSubscribe: 1, unsubscriptionMode: 4, // (manual unsubscribe) + customFields: [] }, l6: { id: 6, cid: 'HyveEPvWW', publicSubscribe: 0, unsubscriptionMode: 0, // (one-step, no form) + customFields: [] } }, settings: { 'service-url': 'http://localhost:' + config.www.port + '/', - 'admin-email': 'admin@example.com', + 'admin-email': 'keep.admin@mailtrain.org', 'default-homepage': 'https://mailtrain.org', 'smtp-hostname': config.testserver.host, 'smtp-port': config.testserver.port, diff --git a/test/e2e/lib/page.js b/test/e2e/lib/page.js index 3a79f014..d201948f 100644 --- a/test/e2e/lib/page.js +++ b/test/e2e/lib/page.js @@ -14,7 +14,7 @@ module.exports = (...extras) => Object.assign({ elements: {}, async getElement(key) { - return await driver.findElement(By.css(this.elements[key])); + return await driver.findElement(By.css(this.elements[key] || key)); }, async getLinkParams(key) { @@ -38,7 +38,7 @@ module.exports = (...extras) => Object.assign({ if (selector) { await driver.wait(until.elementLocated(By.css(selector)), waitTimeout); } - + for (const elem of (this.elementsToWaitFor || [])) { const sel = this.elements[elem]; if (!sel) { diff --git a/test/e2e/page-objects/subscription.js b/test/e2e/page-objects/subscription.js index 251e92b9..34a389c9 100644 --- a/test/e2e/page-objects/subscription.js +++ b/test/e2e/page-objects/subscription.js @@ -3,6 +3,49 @@ const config = require('../lib/config'); const web = require('../lib/web'); const mail = require('../lib/mail'); +const expect = require('chai').expect; + +const fieldHelpers = list => ({ + async fillFields(subscription) { + if (subscription.EMAIL && this.url === `/subscription/${list.cid}`) { + await this.setValue('emailInput', subscription.EMAIL); + } + + if (subscription.FIRST_NAME) { + await this.setValue('firstNameInput', subscription.FIRST_NAME); + } + + if (subscription.LAST_NAME) { + await this.setValue('lastNameInput', subscription.LAST_NAME); + } + + for (const field of list.customFields) { + if (field.key in subscription) { + await this.setValue(`[name="${field.column}"]`, subscription[field.key]); + } + } + }, + + async assertFields(subscription) { + if (subscription.EMAIL) { + expect(await this.getValue('emailInput')).to.equal(subscription.EMAIL); + } + + if (subscription.FIRST_NAME) { + expect(await this.getValue('firstNameInput')).to.equal(subscription.FIRST_NAME); + } + + if (subscription.LAST_NAME) { + expect(await this.getValue('lastNameInput')).to.equal(subscription.LAST_NAME); + } + + for (const field of list.customFields) { + if (field.key in subscription) { + expect(await this.getValue(`[name="${field.column}"]`)).to.equal(subscription[field.key]); + } + } + } +}); module.exports = list => ({ @@ -17,7 +60,7 @@ module.exports = list => ({ lastNameInput: '#main-form input[name="last-name"]', submitButton: 'a[href="#submit"]' } - }), + }, fieldHelpers(list)), webSubscribeNonPublic: web({ url: `/subscription/${list.cid}`, @@ -83,7 +126,7 @@ module.exports = list => ({ links: { manageAddressLink: `/subscription/${list.cid}/manage-address/:ucid` } - }), + }, fieldHelpers(list)), webManageAddress: web({ url: `/subscription/${list.cid}/manage-address/:ucid`, diff --git a/test/e2e/tests/subscription.js b/test/e2e/tests/subscription.js index ef81bcda..040cdcc2 100644 --- a/test/e2e/tests/subscription.js +++ b/test/e2e/tests/subscription.js @@ -7,6 +7,8 @@ const { useCase, step, precondition, driver } = require('../lib/mocha-e2e'); const shortid = require('shortid'); const expect = require('chai').expect; const createPage = require('../page-objects/subscription'); +const faker = require('faker'); +const request = require('request-promise'); function getPage(listConf) { return createPage(listConf); @@ -16,6 +18,59 @@ function generateEmail() { return 'keep.' + shortid.generate() + '@mailtrain.org'; } +function generateCustomFieldValue(field) { + // https://github.com/marak/Faker.js/#api-methods + switch (field.type) { + case 'text': + return faker.lorem.words(); + case 'website': + return faker.internet.url(); + case 'gpg': + return ''; + case 'longtext': + return faker.lorem.lines(); + case 'json': + return `{"say":"${faker.lorem.word()}"}`; + case 'number': + return faker.random.number().toString(); + case 'option': + return Math.round(Math.random()); + case 'date-us': + return '10/20/2017'; + case 'date-eur': + return '20/10/2017'; + case 'birthday-us': + return '10/20'; + case 'birthday-eur': + return '20/10'; + default: + return ''; + } +} + +function generateSubscriptionData(listConf) { + const data = { + EMAIL: generateEmail(), + FIRST_NAME: faker.name.firstName(), + LAST_NAME: faker.name.lastName(), + TIMEZONE: 'Europe/Tallinn', + }; + + listConf.customFields.forEach(field => { + data[field.key] = generateCustomFieldValue(field); + }); + + return data; +} + +function changeSubscriptionData(listConf, subscription) { + const data = generateSubscriptionData(listConf); + delete data.EMAIL; + const changedSubscription = Object.assign({}, subscription, data); + // TODO: Make sure values have actually changed. + return changedSubscription; +} + async function subscribe(listConf, subscription) { const page = getPage(listConf); @@ -24,16 +79,7 @@ async function subscribe(listConf, subscription) { }); await step('User submits a valid email and other subscription info.', async () => { - await page.webSubscribe.setValue('emailInput', subscription.email); - - if (subscription.firstName) { - await page.webSubscribe.setValue('firstNameInput', subscription.firstName); - } - - if (subscription.lastName) { - await page.webSubscribe.setValue('lastNameInput', subscription.lastName); - } - + await page.webSubscribe.fillFields(subscription); await page.webSubscribe.submit(); }); @@ -42,7 +88,7 @@ async function subscribe(listConf, subscription) { }); await step('System sends an email with a link to confirm the subscription.', async () => { - await page.mailConfirmSubscription.fetchMail(subscription.email); + await page.mailConfirmSubscription.fetchMail(subscription.EMAIL); }); await step('User clicks confirm subscription in the email', async () => { @@ -54,7 +100,7 @@ async function subscribe(listConf, subscription) { }); await step('System sends an email with subscription confirmation.', async () => { - await page.mailSubscriptionConfirmed.fetchMail(subscription.email); + await page.mailSubscriptionConfirmed.fetchMail(subscription.EMAIL); subscription.unsubscribeLink = await page.mailSubscriptionConfirmed.getHref('unsubscribeLink'); subscription.manageLink = await page.mailSubscriptionConfirmed.getHref('manageLink'); @@ -79,7 +125,7 @@ suite('Subscription use-cases', () => { useCase('Subscription to a public list (main scenario)', async () => { await subscribe(config.lists.l1, { - email: generateEmail() + EMAIL: generateEmail() }); }); @@ -105,7 +151,7 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l1); const subscription = await subscriptionExistsPrecondition(config.lists.l1, { - email: generateEmail() + EMAIL: generateEmail() }); await step('User navigates to list subscribe page', async () => { @@ -113,7 +159,7 @@ suite('Subscription use-cases', () => { }); await step('User submits the email which has been already registered.', async () => { - await page.webSubscribe.setValue('emailInput', subscription.email); + await page.webSubscribe.setValue('emailInput', subscription.EMAIL); await page.webSubscribe.submit(); }); @@ -122,7 +168,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email informing that the address has been already registered.', async () => { - await page.mailAlreadySubscribed.fetchMail(subscription.email); + await page.mailAlreadySubscribed.fetchMail(subscription.EMAIL); }); }); @@ -138,11 +184,7 @@ suite('Subscription use-cases', () => { useCase('Change profile info', async () => { const page = getPage(config.lists.l1); - const subscription = await subscriptionExistsPrecondition(config.lists.l1, { - email: generateEmail(), - firstName: 'John', - lastName: 'Doe' - }); + let subscription = await subscriptionExistsPrecondition(config.lists.l1, generateSubscriptionData(config.lists.l1)); await step('User clicks the manage subscription button.', async () => { await page.mailSubscriptionConfirmed.click('manageLink'); @@ -150,16 +192,12 @@ suite('Subscription use-cases', () => { await step('Systems shows a form to change subscription details. The form contains data entered during subscription.', async () => { await page.webManage.waitUntilVisibleAfterRefresh(); - expect(await page.webManage.getValue('emailInput')).to.equal(subscription.email); - expect(await page.webManage.getValue('firstNameInput')).to.equal(subscription.firstName); - expect(await page.webManage.getValue('lastNameInput')).to.equal(subscription.lastName); + await page.webManage.assertFields(subscription); }); - await step('User enters another name and submits the form.', async () => { - subscription.firstName = 'Adam'; - subscription.lastName = 'B'; - await page.webManage.setValue('firstNameInput', subscription.firstName); - await page.webManage.setValue('lastNameInput', subscription.lastName); + await step('User enters other values and submits the form.', async () => { + subscription = changeSubscriptionData(config.lists.l1, subscription); + await page.webManage.fillFields(subscription); await page.webManage.submit(); }); @@ -168,14 +206,11 @@ suite('Subscription use-cases', () => { }); await step('User navigates to manage subscription again.', async () => { - // await page.webManage.navigate(subscription.manageLink); await page.webManage.navigate({ ucid: subscription.ucid }); }); await step('Systems shows a form with the changes made previously.', async () => { - expect(await page.webManage.getValue('emailInput')).to.equal(subscription.email); - expect(await page.webManage.getValue('firstNameInput')).to.equal(subscription.firstName); - expect(await page.webManage.getValue('lastNameInput')).to.equal(subscription.lastName); + await page.webManage.assertFields(subscription); }); }); @@ -183,9 +218,9 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l1); const subscription = await subscriptionExistsPrecondition(config.lists.l1, { - email: generateEmail(), - firstName: 'John', - lastName: 'Doe' + EMAIL: generateEmail(), + FIRST_NAME: 'John', + LAST_NAME: 'Doe' }); await step('User clicks the manage subscription button.', async () => { @@ -194,9 +229,7 @@ suite('Subscription use-cases', () => { await step('Systems shows a form to change subscription details. The form contains data entered during subscription.', async () => { await page.webManage.waitUntilVisibleAfterRefresh(); - expect(await page.webManage.getValue('emailInput')).to.equal(subscription.email); - expect(await page.webManage.getValue('firstNameInput')).to.equal(subscription.firstName); - expect(await page.webManage.getValue('lastNameInput')).to.equal(subscription.lastName); + await page.webManage.fillFields(subscription); }); await step('User clicks the change address button.', async () => { @@ -208,8 +241,8 @@ suite('Subscription use-cases', () => { }); await step('User fills in a new email address and submits the form.', async () => { - subscription.email = generateEmail(); - await page.webManageAddress.setValue('emailNewInput', subscription.email); + subscription.EMAIL = generateEmail(); + await page.webManageAddress.setValue('emailNewInput', subscription.EMAIL); await page.webManageAddress.submit(); }); @@ -220,7 +253,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email with a link to confirm the address change.', async () => { - await page.mailConfirmAddressChange.fetchMail(subscription.email); + await page.mailConfirmAddressChange.fetchMail(subscription.EMAIL); }); await step('User clicks confirm subscription in the email', async () => { @@ -231,11 +264,11 @@ suite('Subscription use-cases', () => { await page.webManage.waitUntilVisibleAfterRefresh(); await page.webManage.waitForFlash(); expect(await page.webManage.getFlash()).to.contain('Email address changed'); - expect(await page.webManage.getValue('emailInput')).to.equal(subscription.email); + expect(await page.webManage.getValue('emailInput')).to.equal(subscription.EMAIL); }); await step('System sends an email with subscription confirmation.', async () => { - await page.mailSubscriptionConfirmed.fetchMail(subscription.email); + await page.mailSubscriptionConfirmed.fetchMail(subscription.EMAIL); }); }); @@ -243,7 +276,7 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l1); const subscription = await subscriptionExistsPrecondition(config.lists.l1, { - email: generateEmail() + EMAIL: generateEmail() }); await step('User clicks the unsubscribe button.', async () => { @@ -255,7 +288,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email that confirms unsubscription.', async () => { - await page.mailUnsubscriptionConfirmed.fetchMail(subscription.email); + await page.mailUnsubscriptionConfirmed.fetchMail(subscription.EMAIL); }); }); @@ -263,7 +296,7 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l2); const subscription = await subscriptionExistsPrecondition(config.lists.l2, { - email: generateEmail() + EMAIL: generateEmail() }); await step('User clicks the unsubscribe button.', async () => { @@ -283,7 +316,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email that confirms unsubscription.', async () => { - await page.mailUnsubscriptionConfirmed.fetchMail(subscription.email); + await page.mailUnsubscriptionConfirmed.fetchMail(subscription.EMAIL); }); }); @@ -291,7 +324,7 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l3); const subscription = await subscriptionExistsPrecondition(config.lists.l3, { - email: generateEmail() + EMAIL: generateEmail() }); await step('User clicks the unsubscribe button.', async () => { @@ -303,7 +336,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email with a link to confirm unsubscription.', async () => { - await page.mailConfirmUnsubscription.fetchMail(subscription.email); + await page.mailConfirmUnsubscription.fetchMail(subscription.EMAIL); }); await step('User clicks the confirm unsubscribe button in the email.', async () => { @@ -315,7 +348,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email that confirms unsubscription.', async () => { - await page.mailUnsubscriptionConfirmed.fetchMail(subscription.email); + await page.mailUnsubscriptionConfirmed.fetchMail(subscription.EMAIL); }); }); @@ -323,7 +356,7 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l4); const subscription = await subscriptionExistsPrecondition(config.lists.l4, { - email: generateEmail() + EMAIL: generateEmail() }); await step('User clicks the unsubscribe button.', async () => { @@ -343,7 +376,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email with a link to confirm unsubscription.', async () => { - await page.mailConfirmUnsubscription.fetchMail(subscription.email); + await page.mailConfirmUnsubscription.fetchMail(subscription.EMAIL); }); await step('User clicks the confirm unsubscribe button in the email.', async () => { @@ -355,7 +388,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email that confirms unsubscription.', async () => { - await page.mailUnsubscriptionConfirmed.fetchMail(subscription.email); + await page.mailUnsubscriptionConfirmed.fetchMail(subscription.EMAIL); }); }); @@ -363,7 +396,7 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l5); await subscriptionExistsPrecondition(config.lists.l5, { - email: generateEmail() + EMAIL: generateEmail() }); await step('User clicks the unsubscribe button.', async () => { @@ -379,9 +412,9 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l1); const subscription = await subscriptionExistsPrecondition(config.lists.l1, { - email: generateEmail(), - firstName: 'John', - lastName: 'Doe' + EMAIL: generateEmail(), + FIRST_NAME: 'John', + LAST_NAME: 'Doe' }); await step('User clicks the unsubscribe button.', async () => { @@ -393,7 +426,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email that confirms unsubscription.', async () => { - await page.mailUnsubscriptionConfirmed.fetchMail(subscription.email); + await page.mailUnsubscriptionConfirmed.fetchMail(subscription.EMAIL); }); await step('User clicks the resubscribe button.', async () => { @@ -402,9 +435,9 @@ suite('Subscription use-cases', () => { await step('Systems shows the subscription form. The form contains data entered during initial subscription.', async () => { await page.webSubscribe.waitUntilVisibleAfterRefresh(); - expect(await page.webSubscribe.getValue('emailInput')).to.equal(subscription.email); - expect(await page.webSubscribe.getValue('firstNameInput')).to.equal(subscription.firstName); - expect(await page.webSubscribe.getValue('lastNameInput')).to.equal(subscription.lastName); + expect(await page.webSubscribe.getValue('emailInput')).to.equal(subscription.EMAIL); + expect(await page.webSubscribe.getValue('firstNameInput')).to.equal(subscription.FIRST_NAME); + expect(await page.webSubscribe.getValue('lastNameInput')).to.equal(subscription.LAST_NAME); }); await step('User submits the subscription form.', async () => { @@ -416,7 +449,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email with a link to confirm the subscription.', async () => { - await page.mailConfirmSubscription.fetchMail(subscription.email); + await page.mailConfirmSubscription.fetchMail(subscription.EMAIL); }); await step('User clicks confirm subscription in the email', async () => { @@ -428,7 +461,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email with subscription confirmation. The manage and unsubscribe links are identical with the initial subscription.', async () => { - await page.mailSubscriptionConfirmed.fetchMail(subscription.email); + await page.mailSubscriptionConfirmed.fetchMail(subscription.EMAIL); const unsubscribeLink = await page.mailSubscriptionConfirmed.getHref('unsubscribeLink'); const manageLink = await page.mailSubscriptionConfirmed.getHref('manageLink'); expect(subscription.unsubscribeLink).to.equal(unsubscribeLink); @@ -440,9 +473,9 @@ suite('Subscription use-cases', () => { const page = getPage(config.lists.l1); const oldSubscription = await subscriptionExistsPrecondition(config.lists.l1, { - email: generateEmail(), - firstName: 'old first name', - lastName: 'old last name' + EMAIL: generateEmail(), + FIRST_NAME: 'old first name', + LAST_NAME: 'old last name' }); await step('User clicks the unsubscribe button.', async () => { @@ -454,12 +487,12 @@ suite('Subscription use-cases', () => { }); await step('System sends an email that confirms unsubscription.', async () => { - await page.mailUnsubscriptionConfirmed.fetchMail(oldSubscription.email); + await page.mailUnsubscriptionConfirmed.fetchMail(oldSubscription.EMAIL); }); const newSubscription = await subscriptionExistsPrecondition(config.lists.l1, { - email: generateEmail(), - firstName: 'new first name' + EMAIL: generateEmail(), + FIRST_NAME: 'new first name' }); await step('User clicks the manage subscription button.', async () => { @@ -467,6 +500,7 @@ suite('Subscription use-cases', () => { }); await step('User clicks the change address button.', async () => { + await page.webManage.waitUntilVisibleAfterRefresh(); await page.webManage.click('manageAddressLink'); }); @@ -475,7 +509,7 @@ suite('Subscription use-cases', () => { }); await step('User fills in the email address of the original subscription and submits the form.', async () => { - await page.webManageAddress.setValue('emailNewInput', oldSubscription.email); + await page.webManageAddress.setValue('emailNewInput', oldSubscription.EMAIL); await page.webManageAddress.submit(); }); @@ -486,7 +520,7 @@ suite('Subscription use-cases', () => { }); await step('System sends an email with a link to confirm the address change.', async () => { - await page.mailConfirmAddressChange.fetchMail(oldSubscription.email); + await page.mailConfirmAddressChange.fetchMail(oldSubscription.EMAIL); }); await step('User clicks confirm subscription in the email', async () => { @@ -497,13 +531,117 @@ suite('Subscription use-cases', () => { await page.webManage.waitUntilVisibleAfterRefresh(); await page.webManage.waitForFlash(); expect(await page.webManage.getFlash()).to.contain('Email address changed'); - expect(await page.webManage.getValue('emailInput')).to.equal(oldSubscription.email); - expect(await page.webManage.getValue('firstNameInput')).to.equal(newSubscription.firstName); + expect(await page.webManage.getValue('emailInput')).to.equal(oldSubscription.EMAIL); + expect(await page.webManage.getValue('firstNameInput')).to.equal(newSubscription.FIRST_NAME); expect(await page.webManage.getValue('lastNameInput')).to.equal(''); }); await step('System sends an email with subscription confirmation.', async () => { - await page.mailSubscriptionConfirmed.fetchMail(oldSubscription.email); + await page.mailSubscriptionConfirmed.fetchMail(oldSubscription.EMAIL); + }); + }); + +}); + + +async function apiSubscribe(listConf, subscription) { + await step('Add subscription via API call.', async () => { + const response = await request({ + uri: `${config.baseUrl}/api/subscribe/${listConf.cid}?access_token=${config.users.admin.accessToken}`, + method: 'POST', + json: subscription + }); + expect(response.error).to.be.a('undefined'); + expect(response.data.id).to.be.a('string'); + subscription.ucid = response.data.id; + }); + return subscription; +} + +suite('API Subscription use-cases', () => { + + useCase('Subscription to list #1, without confirmation.', async () => { + const page = getPage(config.lists.l1); + const subscription = await apiSubscribe(config.lists.l1, generateSubscriptionData(config.lists.l1)); + + await step('User navigates to manage subscription.', async () => { + await page.webManage.navigate({ ucid: subscription.ucid }); + }); + + await step('Systems shows a form containing the data submitted with the API call.', async () => { + await page.webManage.assertFields(subscription); + }); + }); + + useCase('Subscription to list #1, with confirmation.', async () => { + const page = getPage(config.lists.l1); + + const subscription = await apiSubscribe(config.lists.l1, Object.assign(generateSubscriptionData(config.lists.l1), { + REQUIRE_CONFIRMATION: 'yes' + })); + + await step('System sends an email with a link to confirm the subscription.', async () => { + await page.mailConfirmSubscription.fetchMail(subscription.EMAIL); + }); + + await step('User clicks confirm subscription in the email', async () => { + await page.mailConfirmSubscription.click('confirmLink'); + }); + + await step('System shows a notice that subscription has been confirmed.', async () => { + await page.webSubscribedNotice.waitUntilVisibleAfterRefresh(); + }); + + await step('System sends an email with subscription confirmation.', async () => { + await page.mailSubscriptionConfirmed.fetchMail(subscription.EMAIL); + }); + + await step('User navigates to manage subscription.', async () => { + await page.webManage.navigate({ ucid: subscription.ucid }); + }); + + await step('Systems shows a form containing the data submitted with the API call.', async () => { + await page.webManage.assertFields(subscription); + }); + }); + + useCase('Change profile info', async () => { + const page = getPage(config.lists.l1); + + const initialSubscription = await apiSubscribe(config.lists.l1, generateSubscriptionData(config.lists.l1)); + + const update = changeSubscriptionData(config.lists.l1, initialSubscription); + delete update.FIRST_NAME; + + const changedSubscription = await apiSubscribe(config.lists.l1, update); + changedSubscription.FIRST_NAME = initialSubscription.FIRST_NAME; + + expect(changedSubscription.ucid).to.equal(initialSubscription.ucid); + + await step('User navigates to manage subscription.', async () => { + await page.webManage.navigate({ ucid: changedSubscription.ucid }); + }); + + await step('Systems shows a form containing the updated subscription data.', async () => { + await page.webManage.assertFields(changedSubscription); + }); + }); + + useCase('Unsubscribe', async () => { + const subscription = await apiSubscribe(config.lists.l1, generateSubscriptionData(config.lists.l1)); + + await step('Unsubsribe via API call.', async () => { + const response = await request({ + uri: `${config.baseUrl}/api/unsubscribe/${config.lists.l1.cid}?access_token=${config.users.admin.accessToken}`, + method: 'POST', + json: { + EMAIL: subscription.EMAIL + } + }); + + expect(response.error).to.be.a('undefined'); + expect(response.data.id).to.be.a('number'); // FIXME Shouldn't data.id be the cid instead of the DB id? + expect(response.data.unsubscribed).to.equal(true); }); }); From 1d76eefe273af2439972f87f3c4b801573e5d064 Mon Sep 17 00:00:00 2001 From: witzig Date: Wed, 14 Jun 2017 11:07:16 +0200 Subject: [PATCH 05/12] Fixed unhandled promise rejection #254 (and some cleanup) --- routes/editorapi.js | 62 +++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/routes/editorapi.js b/routes/editorapi.js index fd85bc73..e927163d 100644 --- a/routes/editorapi.js +++ b/routes/editorapi.js @@ -1,5 +1,6 @@ 'use strict'; +let log = require('npmlog'); let config = require('config'); let express = require('express'); let router = new express.Router(); @@ -70,19 +71,15 @@ let listImages = (dir, dirURL, callback) => { }); }; -let getStaticImageUrl = ({ - dynamicUrl, - staticDir, - staticDirUrl -}, callback) => { +const getStaticImageUrl = (dynamicUrl, staticDir, staticDirUrl, callback) => { mkdirp(staticDir, err => { if (err) { - return callback(dynamicUrl); + return callback(err); } fs.readdir(staticDir, (err, files) => { if (err) { - return callback(dynamicUrl); + return callback(err); } let hash = crypto.createHash('md5').update(dynamicUrl).digest('hex'); @@ -90,7 +87,7 @@ let getStaticImageUrl = ({ let headers = {}; if (match) { - return callback(staticDirUrl + '/' + match); + return callback(null, staticDirUrl + '/' + match); } if (dynamicUrl.includes('/editorapi/img?')) { @@ -103,24 +100,27 @@ let getStaticImageUrl = ({ headers }) .then(res => { + if (res.status < 200 || res.status >= 300) { + throw new Error(`Received HTTP status code ${res.status} while fetching image ${dynamicUrl}`); + } return res.buffer(); }) .then(buffer => { let ft = fileType(buffer); - if (!ft) { - return callback(dynamicUrl); - } - if (['image/jpeg', 'image/png', 'image/gif'].includes(ft.mime)) { + if (ft && ['image/jpeg', 'image/png', 'image/gif'].includes(ft.mime)) { fs.writeFile(path.join(staticDir, hash + '.' + ft.ext), buffer, err => { if (err) { - return callback(dynamicUrl); + throw err; } let staticUrl = staticDirUrl + '/' + hash + '.' + ft.ext; - callback(staticUrl); + callback(null, staticUrl); }); } else { - callback(dynamicUrl); + throw new Error(`Unsupported image MIME type for ${dynamicUrl}`); } + }) + .catch(err => { + callback(err); }); }); }); @@ -139,6 +139,7 @@ let prepareHtml = ({ let srcs = {}; let re = /]+src="([^"]+)"/g; let result; + while ((result = re.exec(html)) !== null) { srcs[result[1]] = result[1]; } @@ -153,15 +154,26 @@ let prepareHtml = ({ } }; + const staticDir = path.join(__dirname, '..', 'public', editorName, 'uploads', 'static'); + const staticDirUrl = url.resolve(serviceUrl, editorName + '/uploads/static'); + Object.keys(srcs).forEach(src => { jobs++; let dynamicUrl = src.replace(/&/g, '&'); dynamicUrl = /^https?:\/\/|^\/\//i.test(dynamicUrl) ? dynamicUrl : url.resolve(serviceUrl, dynamicUrl); - getStaticImageUrl({ - dynamicUrl, - staticDir: path.join(__dirname, '..', 'public', editorName, 'uploads', 'static'), - staticDirUrl: url.resolve(serviceUrl, editorName + '/uploads/static'), - }, staticUrl => { + + getStaticImageUrl(dynamicUrl, staticDir, staticDirUrl, (err, staticUrl) => { + if (err) { + // TODO: Send a warning back to the editor. For now we just skip image resizing. + log.error('editorapi', err.message || err); + + if (dynamicUrl.includes('/editorapi/img?')) { + staticUrl = url.parse(dynamicUrl, true).query.src || dynamicUrl; + } else { + staticUrl = dynamicUrl; + } + } + srcs[src] = staticUrl; jobs--; done(); @@ -329,10 +341,12 @@ router.get('/upload', passport.csrfProtection, (req, res) => { if (req.query.type === 'campaign' && Number(req.query.id) > 0) { listImages(path.join(baseDir, req.query.id), baseDirUrl + '/' + req.query.id, (err, campaignImages) => { - err ? res.status(500).send(err.message || err) : - res.json({ - files: sharedImages.concat(campaignImages) - }); + if (err) { + return res.status(500).send(err.message || err); + } + res.json({ + files: sharedImages.concat(campaignImages) + }); }); } else { res.json({ From 4f5c132db4bca35e0fb310ce1c03da73cd9e9170 Mon Sep 17 00:00:00 2001 From: witzig Date: Thu, 15 Jun 2017 19:11:13 +0200 Subject: [PATCH 06/12] Fixed bug: Hidden custom fields (for custom forms) losing values when updating preferences. Credits to @flapuente-palbin --- lib/models/subscriptions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/subscriptions.js b/lib/models/subscriptions.js index 6760bda9..f9b562c5 100644 --- a/lib/models/subscriptions.js +++ b/lib/models/subscriptions.js @@ -414,7 +414,7 @@ module.exports.update = (listId, cid, updates, allowEmail, callback) => { } }); - fields.getValues(fields.getRow(fieldList, updates, true, true), true).forEach(field => { + fields.getValues(fields.getRow(fieldList, updates, true, true, true), true).forEach(field => { keys.push(field.key); values.push(field.value); }); From 1db7bd987355ba95365d3d4832cd2711b94d6c68 Mon Sep 17 00:00:00 2001 From: witzig Date: Thu, 15 Jun 2017 21:23:35 +0200 Subject: [PATCH 07/12] Bumped deps --- package.json | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index e466e96b..c0a84364 100644 --- a/package.json +++ b/package.json @@ -32,15 +32,15 @@ }, "devDependencies": { "babel-eslint": "^7.2.3", - "chai": "^3.5.0", - "eslint-config-nodemailer": "^1.0.0", + "chai": "^4.0.2", + "eslint-config-nodemailer": "^1.2.0", "grunt": "^1.0.1", "grunt-cli": "^1.2.0", "grunt-contrib-nodeunit": "^1.0.0", - "grunt-eslint": "^19.0.0", + "grunt-eslint": "^20.0.0", "jsxgettext-andris": "^0.9.0-patch.1", "mocha": "^3.3.0", - "phantomjs": "^2.1.7", + "phantomjs-prebuilt": "^2.1.14", "selenium-webdriver": "^3.4.0", "url-pattern": "^1.0.3" }, @@ -63,13 +63,13 @@ "csv-generate": "^1.0.0", "csv-parse": "^1.2.0", "device": "^0.3.8", - "dompurify": "^0.8.5", + "dompurify": "^0.9.0", "escape-html": "^1.0.3", "express": "^4.15.2", "express-session": "^1.15.2", "faker": "^4.1.0", "feedparser": "^2.1.0", - "file-type": "^4.1.0", + "file-type": "^5.2.0", "fs-extra": "^3.0.1", "geoip-ultralight": "^0.1.5", "gettext-parser": "^1.2.2", @@ -89,7 +89,7 @@ "mailparser": "^2.0.5", "marked": "^0.3.6", "memory-cache": "^0.1.6", - "mjml": "3.3.0", + "mjml": "3.3.3", "mkdirp": "^0.5.1", "moment-timezone": "^0.5.12", "morgan": "^1.8.1", @@ -98,7 +98,7 @@ "mysql": "^2.13.0", "node-gettext": "^2.0.0-rc.1", "node-mocks-http": "^1.6.1", - "nodemailer": "^3.1.8", + "nodemailer": "^4.0.1", "nodemailer-openpgp": "^1.0.2", "npmlog": "^4.0.2", "object-hash": "^1.1.7", @@ -113,7 +113,7 @@ "serve-favicon": "^2.4.2", "shortid": "^2.2.8", "slugify": "^1.1.0", - "smtp-server": "^2.0.3", + "smtp-server": "^3.0.1", "striptags": "^3.0.1", "toml": "^2.3.2", "try-require": "^1.2.1" From 2da90b58e494025eec90d34b6695fbcb1d90b3f9 Mon Sep 17 00:00:00 2001 From: witzig Date: Thu, 15 Jun 2017 21:26:35 +0200 Subject: [PATCH 08/12] Satisfy new eslint indentation rule --- lib/db.js | 6 +++--- lib/file-helpers.js | 10 +++++----- lib/models/fields.js | 2 ++ lib/tools.js | 18 +++++++++--------- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/db.js b/lib/db.js index 8d6fe5db..44f17b0f 100644 --- a/lib/db.js +++ b/lib/db.js @@ -43,9 +43,9 @@ if (config.redis && config.redis.enabled) { return setImmediate(() => callback()); } module.exports.redis.multi(). - lpush('mailtrain:cache:' + key, stringifyDate.stringify(value)). - expire('mailtrain:cache:' + key, 24 * 3600). - exec(err => callback(err)); + lpush('mailtrain:cache:' + key, stringifyDate.stringify(value)). + expire('mailtrain:cache:' + key, 24 * 3600). + exec(err => callback(err)); }; module.exports.getFromCache = (key, callback) => { diff --git a/lib/file-helpers.js b/lib/file-helpers.js index fdad5caf..deee31e1 100644 --- a/lib/file-helpers.js +++ b/lib/file-helpers.js @@ -4,11 +4,11 @@ const path = require('path'); function nameToFileName(name) { return name. - trim(). - toLowerCase(). - replace(/[ .+/]/g, '-'). - replace(/[^a-z0-9\-_]/gi, ''). - replace(/--*/g, '-'); + trim(). + toLowerCase(). + replace(/[ .+/]/g, '-'). + replace(/[^a-z0-9\-_]/gi, ''). + replace(/--*/g, '-'); } diff --git a/lib/models/fields.js b/lib/models/fields.js index 0e955ad8..bda2c1b2 100644 --- a/lib/models/fields.js +++ b/lib/models/fields.js @@ -398,6 +398,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { // ignore missing values return; } + /* eslint-disable indent */ switch (field.type) { case 'text': case 'website': @@ -574,6 +575,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { break; } } + /* eslint-enable indent */ }); return row; diff --git a/lib/tools.js b/lib/tools.js index 18787fc5..89ac47aa 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -35,12 +35,12 @@ module.exports = { function toDbKey(key) { return key. - replace(/[^a-z0-9\-_]/gi, ''). - replace(/\-+/g, '_'). - replace(/[A-Z]/g, c => '_' + c.toLowerCase()). - replace(/^_+|_+$/g, ''). - replace(/_+/g, '_'). - trim(); + replace(/[^a-z0-9\-_]/gi, ''). + replace(/\-+/g, '_'). + replace(/[A-Z]/g, c => '_' + c.toLowerCase()). + replace(/^_+|_+$/g, ''). + replace(/_+/g, '_'). + trim(); } function fromDbKey(key) { @@ -71,9 +71,9 @@ function convertKeys(obj, options) { function queryParams(obj) { return Object.keys(obj). - filter(key => key !== '_csrf'). - map(key => encodeURIComponent(key) + '=' + encodeURIComponent(obj[key])). - join('&'); + filter(key => key !== '_csrf'). + map(key => encodeURIComponent(key) + '=' + encodeURIComponent(obj[key])). + join('&'); } function createSlug(table, name, callback) { From 80aca59af7be456dc0080b8db88c6b95ddd5ba7e Mon Sep 17 00:00:00 2001 From: witzig Date: Thu, 15 Jun 2017 21:28:40 +0200 Subject: [PATCH 09/12] Disable eslint rule no-await-in-loop for e2e tests --- test/e2e/.eslintrc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/.eslintrc b/test/e2e/.eslintrc index a5157382..78fe9cc2 100644 --- a/test/e2e/.eslintrc +++ b/test/e2e/.eslintrc @@ -4,7 +4,8 @@ "strict": 0, "no-console": 0, "comma-dangle": 0, - "arrow-body-style": 0 + "arrow-body-style": 0, + "no-await-in-loop": 0 }, "env": { "mocha": true From 830ca4f17f37f7d6e97097bd8497fec792ca1614 Mon Sep 17 00:00:00 2001 From: witzig Date: Thu, 15 Jun 2017 21:44:54 +0200 Subject: [PATCH 10/12] Satisfy eslint rule no-useless-escape --- lib/models/fields.js | 14 +++++++------- lib/tools.js | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/models/fields.js b/lib/models/fields.js index bda2c1b2..b2adeca3 100644 --- a/lib/models/fields.js +++ b/lib/models/fields.js @@ -311,7 +311,7 @@ function addCustomField(listId, name, defaultValue, type, group, groupTemplate, } if (module.exports.grouped.indexOf(type) < 0) { - column = ('custom_' + slugify(name, '_') + '_' + shortid.generate()).toLowerCase().replace(/[^a-z0-9\_]/g, ''); + column = ('custom_' + slugify(name, '_') + '_' + shortid.generate()).toLowerCase().replace(/[^a-z0-9_]/g, ''); } let query = 'INSERT INTO custom_fields (`list`, `name`, `key`,`default_value`, `type`, `group`, `group_template`, `column`, `visible`) VALUES(?,?,?,?,?,?,?,?,?)'; @@ -414,7 +414,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { visible: !!field.visible, mergeTag: field.key, mergeValue: (valueList[field.column] || '').toString().trim() || field.defaultValue, - ['type' + (field.type || '').toString().trim().replace(/(?:^|\-)([a-z])/g, (m, c) => c.toUpperCase())]: true + ['type' + (field.type || '').toString().trim().replace(/(?:^|-)([a-z])/g, (m, c) => c.toUpperCase())]: true }; row.push(item); break; @@ -444,7 +444,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { visible: !!field.visible, mergeTag: field.key, mergeValue: value || field.defaultValue, - ['type' + (field.type || '').toString().trim().replace(/(?:^|\-)([a-z])/g, (m, c) => c.toUpperCase())]: true + ['type' + (field.type || '').toString().trim().replace(/(?:^|-)([a-z])/g, (m, c) => c.toUpperCase())]: true }; row.push(item); break; @@ -460,7 +460,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { visible: !!field.visible, mergeTag: field.key, mergeValue: (Number(valueList[field.column]) || Number(field.defaultValue) || 0).toString(), - ['type' + (field.type || '').toString().trim().replace(/(?:^|\-)([a-z])/g, (m, c) => c.toUpperCase())]: true + ['type' + (field.type || '').toString().trim().replace(/(?:^|-)([a-z])/g, (m, c) => c.toUpperCase())]: true }; row.push(item); break; @@ -477,7 +477,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { key: 'group-g' + field.id, mergeTag: field.key, mergeValue: field.defaultValue, - ['type' + (field.type || '').toString().trim().replace(/(?:^|\-)([a-z])/g, (m, c) => c.toUpperCase())]: true, + ['type' + (field.type || '').toString().trim().replace(/(?:^|-)([a-z])/g, (m, c) => c.toUpperCase())]: true, groupTemplate: field.groupTemplate, options: (field.options || []).map(subField => { if (onlyExisting && subField.column && !valueList.hasOwnProperty(subField.column)) { @@ -508,7 +508,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { case 'date-us': case 'birthday-us': { - let isUs = /\-us$/.test(field.type); + let isUs = /-us$/.test(field.type); let isYear = field.type.indexOf('date-') === 0; let value = valueList[field.column]; let day, month, year; @@ -569,7 +569,7 @@ module.exports.getRow = (fieldList, values, useDate, showAll, onlyExisting) => { visible: !!field.visible, mergeTag: field.key, mergeValue: (useDate ? value : formatted) || field.defaultValue, - ['type' + (field.type || '').toString().trim().replace(/(?:^|\-)([a-z])/g, (m, c) => c.toUpperCase())]: true + ['type' + (field.type || '').toString().trim().replace(/(?:^|-)([a-z])/g, (m, c) => c.toUpperCase())]: true }; row.push(item); break; diff --git a/lib/tools.js b/lib/tools.js index 89ac47aa..ad62a45a 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -36,7 +36,7 @@ module.exports = { function toDbKey(key) { return key. replace(/[^a-z0-9\-_]/gi, ''). - replace(/\-+/g, '_'). + replace(/-+/g, '_'). replace(/[A-Z]/g, c => '_' + c.toLowerCase()). replace(/^_+|_+$/g, ''). replace(/_+/g, '_'). @@ -50,7 +50,7 @@ function fromDbKey(key) { prefix = '_'; } - return prefix + key.replace(/[_\-]([a-z])/g, (m, c) => c.toUpperCase()); + return prefix + key.replace(/[_-]([a-z])/g, (m, c) => c.toUpperCase()); } function convertKeys(obj, options) { From a2ebe8f0f72afe8eede4a92303e48aba4b73892a Mon Sep 17 00:00:00 2001 From: witzig Date: Fri, 16 Jun 2017 02:13:21 +0200 Subject: [PATCH 11/12] Fixed mail-helper.js not calling back when disableConfirmations = true And some refactoring --- lib/subscription-mail-helpers.js | 40 +++++++++++++++----------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/lib/subscription-mail-helpers.js b/lib/subscription-mail-helpers.js index cfbac97e..16e6cdaa 100644 --- a/lib/subscription-mail-helpers.js +++ b/lib/subscription-mail-helpers.js @@ -96,7 +96,7 @@ function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscr } if (!mailOpts.ignoreDisableConfirmations && configItems.disableConfirmations) { - return; + return callback(); } const data = { @@ -110,7 +110,22 @@ function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscr data[relativeUrlKey] = urllib.resolve(configItems.serviceUrl, relativeUrls[relativeUrlKey]); } - function sendMail(html, text) { + let text = { + template: 'subscription/mail-' + template + '-text.hbs' + }; + + let html = { + template: 'subscription/mail-' + template + '-html.mjml.hbs', + layout: 'subscription/layout.mjml.hbs', + type: 'mjml' + }; + + helpers.injectCustomFormTemplates(list.defaultForm, { text, html }, (err, tmpl) => { + if (!err && tmpl) { + text = tmpl.text || text; + html = tmpl.html || html; + } + mailer.sendMail({ from: { name: configItems.defaultFrom, @@ -129,29 +144,12 @@ function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscr }, err => { if (err) { log.error('Subscription', err); + return callback(err); } + callback(); }); - } - let text = { - template: 'subscription/mail-' + template + '-text.hbs' - }; - - let html = { - template: 'subscription/mail-' + template + '-html.mjml.hbs', - layout: 'subscription/layout.mjml.hbs', - type: 'mjml' - }; - - helpers.injectCustomFormTemplates(list.defaultForm, { text, html }, (err, tmpl) => { - if (err) { - return sendMail(html, text); - } - - sendMail(tmpl.html, tmpl.text); }); - - return callback(); }); }); } From 5647dd040d1e64f43b31fd68e2ae9ad409bf4ce4 Mon Sep 17 00:00:00 2001 From: witzig Date: Fri, 16 Jun 2017 14:28:57 +0200 Subject: [PATCH 12/12] Updated subscription-mail-helper.js - Don't wait for mailer to finish. Note: When using encryptionKeys, confirmation redirects feel a bit sluggish. This could probably be 'improved' by calling sendMail via setTimout or fixed by moving sendMail to a worker. --- lib/subscription-mail-helpers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/subscription-mail-helpers.js b/lib/subscription-mail-helpers.js index 16e6cdaa..71ed13af 100644 --- a/lib/subscription-mail-helpers.js +++ b/lib/subscription-mail-helpers.js @@ -144,11 +144,11 @@ function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscr }, err => { if (err) { log.error('Subscription', err); - return callback(err); } - callback(); }); + callback(); + }); }); });