Fixed some bugs in subscription process

Added timezone selector to campaign scheduling
Fixed problems with pausing campaign.
This commit is contained in:
Tomas Bures 2019-07-10 02:06:56 +04:00
parent 4113cb8476
commit e3a5a3c4eb
23 changed files with 218 additions and 99 deletions

View file

@ -1,32 +0,0 @@
'use strict';
const config = require('config');
const knex = require('./lib/knex');
const moment = require('moment');
const shortid = require('shortid');
async function run() {
// const info = await knex('subscription__1').columnInfo();
// console.log(info);
// const ts = moment().toDate();
const ts = new Date(Date.now());
console.log(ts);
const cid = shortid.generate();
await knex('subscription__1')
.insert({
email: cid,
cid,
custom_date_mmddyy_rjkeojrzz: ts
});
const row = await knex('subscription__1').select(['id', 'created', 'custom_date_mmddyy_rjkeojrzz']).where('cid', cid).first();
// const row = await knex('subscription__1').where('id', 2).first();
console.log(row);
}
run();

View file

@ -71,7 +71,7 @@ async function _sendMail(list, email, template, locale, subjectKey, relativeUrls
email
};
const flds = await fields.list(contextHelpers.getAdminContext(), list.id);
const flds = await fields.listGrouped(contextHelpers.getAdminContext(), list.id);
const encryptionKeys = [];
for (const fld of flds) {

View file

@ -124,7 +124,7 @@ async function validateEmail(address) {
function validateEmailGetMessage(result, address, language) {
let t;
if (language) {
if (language !== undefined) {
t = (key, args) => tUI(key, language, args);
} else {
t = (key, args) => tLog(key, args);

View file

@ -339,6 +339,11 @@ async function getTrackingSettingsByCidTx(tx, cid) {
return entity;
}
async function lockByIdTx(tx, id) {
// This locks the entry for update
await tx('campaigns').where('id', id).forUpdate();
}
async function rawGetByTx(tx, key, id) {
const entity = await tx('campaigns').where('campaigns.' + key, id)
.leftJoin('campaign_lists', 'campaigns.id', 'campaign_lists.campaign')
@ -857,8 +862,12 @@ async function getSubscribersQueryGeneratorTx(tx, campaignId) {
}
}
async function _changeStatus(context, campaignId, permittedCurrentStates, newState, invalidStateMessage, startAt) {
async function _changeStatus(context, campaignId, permittedCurrentStates, newState, invalidStateMessage, extraData) {
await knex.transaction(async tx => {
// This is quite inefficient because it selects the same row 3 times. However as status is changed
// rather infrequently, we keep it this way for simplicity
await lockByIdTx(tx, campaignId);
const entity = await getByIdTx(tx, context, campaignId, false);
await enforceSendPermissionTx(tx, context, entity, false);
@ -867,14 +876,36 @@ async function _changeStatus(context, campaignId, permittedCurrentStates, newSta
throw new interoperableErrors.InvalidStateError(invalidStateMessage);
}
if (Array.isArray(newState)) {
const newStateIdx = permittedCurrentStates.indexOf(entity.status);
enforce(newStateIdx != -1);
newState = newState[newStateIdx];
}
const updateData = {
status: newState,
status: newState
};
if (startAt !== undefined) {
if (!extraData) {
updateData.scheduled = null;
updateData.start_at = null;
} else {
const startAt = extraData.startAt;
// If campaign is started without "scheduled" specified, startAt === null
updateData.scheduled = startAt;
if (!startAt || startAt.valueOf() < Date.now()) {
updateData.start_at = new Date();
} else {
updateData.start_at = startAt;
}
const timezone = extraData.timezone;
if (timezone) {
updateData.data = JSON.stringify({
...entity.data,
timezone
});
}
}
@ -887,22 +918,23 @@ async function _changeStatus(context, campaignId, permittedCurrentStates, newSta
}
async function start(context, campaignId, startAt) {
await _changeStatus(context, campaignId, [CampaignStatus.IDLE, CampaignStatus.SCHEDULED, CampaignStatus.PAUSED, CampaignStatus.FINISHED], CampaignStatus.SCHEDULED, 'Cannot start campaign until it is in IDLE, PAUSED, or FINISHED state', startAt);
async function start(context, campaignId, extraData) {
await _changeStatus(context, campaignId, [CampaignStatus.IDLE, CampaignStatus.SCHEDULED, CampaignStatus.PAUSED, CampaignStatus.FINISHED], CampaignStatus.SCHEDULED, 'Cannot start campaign until it is in IDLE, PAUSED, or FINISHED state', extraData);
}
async function stop(context, campaignId) {
await _changeStatus(context, campaignId, [CampaignStatus.SCHEDULED, CampaignStatus.SENDING], CampaignStatus.PAUSING, 'Cannot stop campaign until it is in SCHEDULED or SENDING state');
await _changeStatus(context, campaignId, [CampaignStatus.SCHEDULED, CampaignStatus.SENDING], [CampaignStatus.PAUSED, CampaignStatus.PAUSING], 'Cannot stop campaign until it is in SCHEDULED or SENDING state');
}
async function reset(context, campaignId) {
await knex.transaction(async tx => {
// This is quite inefficient because it selects the same row 3 times. However as RESET is
// going to be called rather infrequently, we keep it this way for simplicity
await lockByIdTx(tx, campaignId);
await shares.enforceEntityPermissionTx(tx, context, 'campaign', campaignId, 'send');
const entity = await tx('campaigns').where('id', campaignId).first();
if (!entity) {
throw new interoperableErrors.NotFoundError();
}
if (entity.status !== CampaignStatus.FINISHED && entity.status !== CampaignStatus.PAUSED) {
throw new interoperableErrors.InvalidStateError('Cannot reset campaign until it is FINISHED or PAUSED state');

View file

@ -841,7 +841,7 @@ async function updateManaged(context, listId, cid, entity) {
for (const key in groupedFieldsMap) {
const fld = groupedFieldsMap[key];
if (fld.order_manage) {
if (fld.order_manage !== null) {
update[key] = entity[key];
}

View file

@ -44,7 +44,7 @@ router.postAsync('/subscribe/:listCid', passport.loggedIn, async (req, res) => {
const emailErr = await tools.validateEmail(input.EMAIL);
if (emailErr) {
const errMsg = tools.validateEmailGetMessage(emailErr, input.email);
const errMsg = tools.validateEmailGetMessage(emailErr, input.email, null);
log.error('API', errMsg);
throw new APIError(errMsg, 400);
}

View file

@ -74,11 +74,13 @@ router.deleteAsync('/campaigns/:campaignId', passport.loggedIn, passport.csrfPro
});
router.postAsync('/campaign-start/:campaignId', passport.loggedIn, passport.csrfProtection, async (req, res) => {
return res.json(await campaigns.start(req.context, castToInteger(req.params.campaignId), null));
return res.json(await campaigns.start(req.context, castToInteger(req.params.campaignId), {startAt: null}));
});
router.postAsync('/campaign-start-at/:campaignId/:dateTime', passport.loggedIn, passport.csrfProtection, async (req, res) => {
return res.json(await campaigns.start(req.context, castToInteger(req.params.campaignId), new Date(Number.parseInt(req.params.dateTime))));
router.postAsync('/campaign-start-at/:campaignId', passport.loggedIn, passport.csrfProtection, async (req, res) => {
const startAt = new Date(req.body.startAt);
const timezone = req.body.timezone;
return res.json(await campaigns.start(req.context, castToInteger(req.params.campaignId), {startAt, timezone}));
});

View file

@ -249,7 +249,7 @@ router.postAsync('/:cid/subscribe', passport.parseForm, corsOrCsrfProtection, as
const emailErr = await tools.validateEmail(email);
if (emailErr) {
const errMsg = tools.validateEmailGetMessage(emailErr, email);
const errMsg = tools.validateEmailGetMessage(emailErr, email, req.locale);
if (req.xhr) {
throw new Error(errMsg);
@ -457,7 +457,7 @@ router.postAsync('/:lcid/manage-address', passport.parseForm, passport.csrfProte
} else {
const emailErr = await tools.validateEmail(emailNew);
if (emailErr) {
const errMsg = tools.validateEmailGetMessage(emailErr, email);
const errMsg = tools.validateEmailGetMessage(emailErr, email, req.locale);
req.flash('danger', errMsg);

View file

@ -328,15 +328,15 @@ async function processCampaign(campaignId) {
while (true) {
const cpg = await knex('campaigns').where('id', campaignId).first();
const expirationThreshold = Date.now() - config.queue.retention.campaign * 1000;
if (cpg.start_at.valueOf() < expirationThreshold) {
return await finish(true, CampaignStatus.FINISHED);
}
if (cpg.status === CampaignStatus.PAUSING) {
return await finish(true, CampaignStatus.PAUSED);
}
const expirationThreshold = Date.now() - config.queue.retention.campaign * 1000;
if (cpg.start_at && cpg.start_at.valueOf() < expirationThreshold) {
return await finish(true, CampaignStatus.FINISHED);
}
sendConfigurationIdByCampaignId.set(cpg.id, cpg.send_configuration);
if (isSendConfigurationPostponed(cpg.send_configuration)) {

View file

@ -13,8 +13,8 @@ if (process.env.NODE_ENV === 'production') {
process.exit(1);
}
if (process.env.NODE_ENV === 'test' && !fs.existsSync(path.join(__dirname, '..', '..', 'config', 'test.toml'))) {
log.error('sqldrop', 'This script only runs in test if config/test.toml (i.e. a dedicated test database) is present');
if (process.env.NODE_ENV === 'test' && !fs.existsSync(path.join(__dirname, '..', '..', 'config', 'test.yaml'))) {
log.error('sqldrop', 'This script only runs in test if config/test.yaml (i.e. a dedicated test database) is present');
process.exit(1);
}

View file

@ -12,8 +12,8 @@ if (process.env.NODE_ENV === 'production') {
process.exit(1);
}
if (process.env.NODE_ENV === 'test' && !fs.existsSync(path.join(__dirname, '..', '..', 'config', 'test.toml'))) {
log.error('sqlinit', 'This script only runs in test if config/test.toml (i.e. a dedicated test database) is present');
if (process.env.NODE_ENV === 'test' && !fs.existsSync(path.join(__dirname, '..', '..', 'config', 'test.yaml'))) {
log.error('sqlinit', 'This script only runs in test if config/test.yaml (i.e. a dedicated test database) is present');
process.exit(1);
}

View file

@ -1,10 +1,11 @@
'use strict';
const config = require('server/test/e2e/lib/config');
const config = require('config');
module.exports = {
app: config,
baseUrl: 'http://localhost:' + config.www.publicPort,
baseTrustedUrl: 'http://localhost:' + config.www.trustedPort,
basePublicUrl: 'http://localhost:' + config.www.publicPort,
mailUrl: 'http://localhost:' + config.testServer.mailboxServerPort,
users: {
admin: {

View file

@ -5,17 +5,13 @@ const log = require('npmlog');
const path = require('path');
const fs = require('fs');
if (process.env.NODE_ENV !== 'test' || !fs.existsSync(path.join(__dirname, '..', '..', '..', 'config', 'test.toml'))) {
log.error('e2e', 'This script only runs in test and config/test.toml (i.e. a dedicated test database) is present');
if (process.env.NODE_ENV !== 'test' || !fs.existsSync(path.join(__dirname, '..', '..', '..', 'config', 'test.yaml'))) {
log.error('e2e', 'This script only runs in test and config/test.yaml (i.e. a dedicated test database) is present');
process.exit(1);
}
if (config.app.testServer.enabled !== true) {
log.error('e2e', 'This script only runs if the testServer is enabled. Check config/test.toml');
log.error('e2e', 'This script only runs if the testServer is enabled. Check config/test.yaml');
process.exit(1);
}
if (config.app.www.port !== 3000) {
log.error('e2e', 'This script requires Mailtrain to be running on port 3000. Check config/test.toml');
process.exit(1);
}

View file

@ -8,7 +8,7 @@ const driver = require('./mocha-e2e').driver;
const url = require('url');
const UrlPattern = require('url-pattern');
const waitTimeout = 10000;
const waitTimeout = 20000;
module.exports = (...extras) => Object.assign({
elements: {},
@ -21,7 +21,8 @@ module.exports = (...extras) => Object.assign({
const elem = await driver.findElement(By.css(this.elements[key]));
const linkUrl = await elem.getAttribute('href');
const linkPath = url.parse(linkUrl).path;
const parsedUrl = url.parse(linkUrl);
const linkPath = parsedUrl.pathname;
const urlPattern = new UrlPattern(this.links[key]);

View file

@ -23,7 +23,7 @@ module.exports = (...extras) => page({
if (parsedUrl.host) {
absolutePath = path;
} else {
absolutePath = config.baseUrl + path;
absolutePath = this.baseUrl + path;
}
await driver.navigate().to(absolutePath);
@ -37,8 +37,8 @@ module.exports = (...extras) => page({
const currentUrl = url.parse(await driver.getCurrentUrl());
const urlPattern = new UrlPattern(desiredUrl);
const params = urlPattern.match(currentUrl.pathname);
if (!params || config.baseUrl !== `${currentUrl.protocol}//${currentUrl.host}`) {
throw new Error(`Unexpected URL. Expecting ${config.baseUrl}${this.url} got ${currentUrl.protocol}//${currentUrl.host}/${currentUrl.pathname}`);
if (!params || this.baseUrl !== `${currentUrl.protocol}//${currentUrl.host}`) {
throw new Error(`Unexpected URL. Expecting ${this.baseUrl}${this.url} got ${currentUrl.protocol}//${currentUrl.host}/${currentUrl.pathname}`);
}
this.params = params;

View file

@ -1,7 +1,9 @@
'use strict';
const config = require('../lib/config');
const web = require('../lib/web');
module.exports = web({
baseUrl: config.baseTrustedUrl,
url: '/'
});

View file

@ -50,6 +50,7 @@ const fieldHelpers = list => ({
module.exports = list => ({
webSubscribe: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}`,
elementsToWaitFor: ['form'],
textsToWaitFor: ['Subscribe to list'],
@ -63,6 +64,7 @@ module.exports = list => ({
}, fieldHelpers(list)),
webSubscribeAfterPost: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/subscribe`,
elementsToWaitFor: ['form'],
textsToWaitFor: ['Subscribe to list'],
@ -76,11 +78,13 @@ module.exports = list => ({
}, fieldHelpers(list)),
webSubscribeNonPublic: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}`,
textsToWaitFor: ['Permission denied'],
}),
webConfirmSubscriptionNotice: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/confirm-subscription-notice`,
textsToWaitFor: ['We need to confirm your email address']
}),
@ -107,6 +111,7 @@ module.exports = list => ({
}),
webSubscribedNotice: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/subscribed-notice`,
textsToWaitFor: ['Subscription Confirmed']
}),
@ -125,6 +130,7 @@ module.exports = list => ({
}),
webManage: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/manage/:ucid`,
elementsToWaitFor: ['form'],
textsToWaitFor: ['Update Your Preferences'],
@ -142,6 +148,7 @@ module.exports = list => ({
}, fieldHelpers(list)),
webManageAddress: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/manage-address/:ucid`,
elementsToWaitFor: ['form'],
textsToWaitFor: ['Update Your Email Address'],
@ -162,11 +169,13 @@ module.exports = list => ({
}),
webUpdatedNotice: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/updated-notice`,
textsToWaitFor: ['Profile Updated']
}),
webUnsubscribedNotice: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/unsubscribed-notice`,
textsToWaitFor: ['Unsubscribe Successful']
}),
@ -180,6 +189,7 @@ module.exports = list => ({
}),
webUnsubscribe: web({
baseUrl: config.basePublicUrl,
elementsToWaitFor: ['submitButton'],
textsToWaitFor: ['Unsubscribe'],
elements: {
@ -188,6 +198,7 @@ module.exports = list => ({
}),
webConfirmUnsubscriptionNotice: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/confirm-unsubscription-notice`,
textsToWaitFor: ['We need to confirm your email address']
}),
@ -201,6 +212,7 @@ module.exports = list => ({
}),
webManualUnsubscribeNotice: web({
baseUrl: config.basePublicUrl,
url: `/subscription/${list.cid}/manual-unsubscribe-notice`,
elementsToWaitFor: ['contactLink'],
textsToWaitFor: ['Online Unsubscription Is Not Possible', config.settings['admin-email']],

View file

@ -1,9 +1,11 @@
'use strict';
const config = require('../lib/config');
const web = require('../lib/web');
module.exports = {
login: web({
baseUrl: config.baseTrustedUrl,
url: '/users/login',
elementsToWaitFor: ['submitButton'],
elements: {
@ -14,11 +16,13 @@ module.exports = {
}),
logout: web({
baseUrl: config.baseTrustedUrl,
requestUrl: '/users/logout',
url: '/'
}),
account: web({
baseUrl: config.baseTrustedUrl,
url: '/users/account',
elementsToWaitFor: ['form'],
elements: {

View file

@ -17,7 +17,7 @@ suite('Login use-cases', () => {
});
test('Anonymous user cannot access restricted content', async () => {
await driver.navigate().to(config.baseUrl + '/settings');
await driver.navigate().to(config.baseTrustedUrl + '/settings');
await page.login.waitUntilVisible();
await page.login.waitForFlash();
expect(await page.login.getFlash()).to.contain('Need to be logged in to access restricted content');

View file

@ -136,7 +136,7 @@ suite('Subscription use-cases', () => {
});
await step('User submits an invalid email.', async () => {
await page.webSubscribe.setValue('emailInput', 'foo@bar.nope');
await page.webSubscribe.setValue('emailInput', 'foo-bar');
await page.webSubscribe.submit();
});
@ -545,7 +545,7 @@ suite('Subscription use-cases', () => {
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}`,
uri: `${config.baseTrustedUrl}/api/subscribe/${listConf.cid}?access_token=${config.users.admin.accessToken}`,
method: 'POST',
json: subscription
});
@ -630,7 +630,7 @@ suite('API Subscription use-cases', () => {
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}`,
uri: `${config.baseTrustedUrl}/api/unsubscribe/${config.lists.l1.cid}?access_token=${config.users.admin.accessToken}`,
method: 'POST',
json: {
EMAIL: subscription.EMAIL