Skip to content
Permalink
Browse files

feat(auth-server): send subscription download email immediately on ac…

…tivation

- move sending of download email from async queue into API route
- clean up unused mailer references in subhub message processing.
- clean up unused oauthdb references in subscription API route

fixes mozilla#2797
  • Loading branch information...
lmorchard committed Oct 8, 2019
1 parent fd1b992 commit 750b54b1eec2602233efc3ad38228033694944f1
@@ -34,7 +34,7 @@ async function run() {
[config.subhubServerMessaging.subhubUpdatesQueueUrl]
);

const [db, translator] = await Promise.all([
const [db] = await Promise.all([
require(`${LIB_DIR}/db`)(config, log, Token).connect(
config[config.db.backend]
),
@@ -44,15 +44,7 @@ async function run() {
),
]);

const { email: mailer } = await require(`${LIB_DIR}/senders`)(
log,
config,
require(`${LIB_DIR}/error`),
translator,
require(`${LIB_DIR}/oauthdb`)(log, config)
);

subhubUpdates(subhubUpdatesQueue, db, mailer);
subhubUpdates(subhubUpdatesQueue, db);
} catch (err) {
log.error('bin.subhub.error', { err });
process.exit(1);
@@ -150,7 +150,7 @@ module.exports = function(
config,
customs,
push,
oauthdb,
mailer,
subhub,
profile
);
@@ -9,7 +9,7 @@ const isA = require('joi');
const ScopeSet = require('../../../fxa-shared').oauth.scopes;
const validators = require('./validators');

module.exports = (log, db, config, customs, push, oauthdb, subhub, profile) => {
module.exports = (log, db, config, customs, push, mailer, subhub, profile) => {
// Skip routes if the subscriptions feature is not configured & enabled
if (!config.subscriptions || !config.subscriptions.enabled) {
return [];
@@ -165,6 +165,12 @@ module.exports = (log, db, config, customs, push, oauthdb, subhub, profile) => {
});
await profile.deleteCache(uid);

const account = await db.account(uid);
await mailer.sendDownloadSubscriptionEmail(account.emails, account, {
acceptLanguage: account.locale,
productId,
});

log.info('subscriptions.createSubscription.success', {
uid,
subscriptionId,
@@ -29,7 +29,7 @@ function validateMessage(message) {
}

module.exports = function(log, config) {
return function start(messageQueue, db, mailer) {
return function start(messageQueue, db) {
async function handleSubHubUpdates(message) {
const uid = message && message.uid;

@@ -63,11 +63,6 @@ module.exports = function(log, config) {
createdAt: message.eventCreatedAt,
});
}
const account = await db.account(uid);
await mailer.sendDownloadSubscriptionEmail(account.emails, account, {
acceptLanguage: account.locale,
productId: message.productName,
});
} else {
if (existing && existing.createdAt >= message.eventCreatedAt) {
suppressNotification = true;
@@ -17,7 +17,7 @@ let config,
db,
customs,
push,
oauthdb,
mailer,
subhub,
profile,
routes,
@@ -28,6 +28,7 @@ let config,
const SUBSCRIPTIONS_MANAGEMENT_SCOPE =
'https://identity.mozilla.com/account/subscriptions';

const ACCOUNT_LOCALE = 'en-US';
const TEST_EMAIL = 'test@email.com';
const UID = uuid.v4('binary').toString('hex');
const NOW = Date.now();
@@ -75,13 +76,6 @@ const DISPLAY_NAME = 'Foo Bar';
const MOCK_CLIENT_ID = '3c49430b43dfba77';
const MOCK_TTL = 3600;
const MOCK_SCOPES = ['profile:email', SUBSCRIPTIONS_MANAGEMENT_SCOPE];
const MOCK_TOKEN_RESPONSE = {
access_token: 'ACCESS',
scope: MOCK_SCOPES,
token_type: 'bearer',
expires_in: MOCK_TTL,
auth_at: 123456,
};

function runTest(routePath, requestOptions) {
routes = require('../../../lib/routes/subscriptions')(
@@ -90,7 +84,7 @@ function runTest(routePath, requestOptions) {
config,
customs,
push,
oauthdb,
mailer,
subhub,
profile
);
@@ -118,6 +112,7 @@ describe('subscriptions', () => {
db = mocks.mockDB({
uid: UID,
email: TEST_EMAIL,
locale: ACCOUNT_LOCALE,
});
db.createAccountSubscription = sinon.spy(async data => ({}));
db.deleteAccountSubscription = sinon.spy(
@@ -134,13 +129,7 @@ describe('subscriptions', () => {
)[0]
);
push = mocks.mockPush();
oauthdb = mocks.mockOAuthDB({
getClientInfo: sinon.spy(async () => {
return { id: MOCK_CLIENT_ID, name: 'mock client' };
}),
grantTokensFromSessionToken: sinon.spy(async () => MOCK_TOKEN_RESPONSE),
});

mailer = mocks.mockMailer();
subhub = mocks.mockSubHub({
getCustomer: sinon.spy(async () => CUSTOMER),
listPlans: sinon.spy(async () => PLANS),
@@ -182,7 +171,7 @@ describe('subscriptions', () => {
config,
customs,
push,
oauthdb,
mailer,
subhub,
profile
);
@@ -299,6 +288,16 @@ describe('subscriptions', () => {
email: TEST_EMAIL,
});
assert.equal(profile.deleteCache.args[0][0], UID);

assert.equal(mailer.sendDownloadSubscriptionEmail.callCount, 1);
const args = mailer.sendDownloadSubscriptionEmail.args[0];
assert.lengthOf(args, 3);
assert.isArray(args[0]);
assert.equal(args[1].uid, UID);
assert.deepEqual(args[2], {
acceptLanguage: ACCOUNT_LOCALE,
productId: PLANS[0].product_id,
});
});

it('should correctly handle payment backend failure on listing plans', async () => {
@@ -8,7 +8,7 @@ const sinon = require('sinon');
const assert = { ...sinon.assert, ...require('chai').assert };

const EventEmitter = require('events').EventEmitter;
const { mockDB, mockLog, mockMailer } = require('../../mocks');
const { mockDB, mockLog } = require('../../mocks');
const subhubUpdates = require('../../../lib/subhub/updates');

const mockDeliveryQueue = new EventEmitter();
@@ -29,15 +29,14 @@ function mockMessage(messageOverrides) {
return message;
}

function mockSubHubUpdates(log, config, db, mailer) {
return subhubUpdates(log, config)(mockDeliveryQueue, db, mailer);
function mockSubHubUpdates(log, config, db) {
return subhubUpdates(log, config)(mockDeliveryQueue, db);
}

describe('subhub updates', () => {
let config;
let db;
let log;
let mailer;

beforeEach(() => {
config = {
@@ -53,23 +52,21 @@ describe('subhub updates', () => {
uid: baseMessage.uid,
});
log = mockLog();
mailer = mockMailer();
});

it('should log validation errors', async () => {
await mockSubHubUpdates(log, config, db, mailer).handleSubHubUpdates(
await mockSubHubUpdates(log, config, db).handleSubHubUpdates(
mockMessage({ subscriptionId: null, active: true })
);
assert.equal(log.error.callCount, 1);
assert.equal(db.createAccountSubscription.callCount, 0);
assert.equal(db.getAccountSubscription.callCount, 0);
assert.equal(db.deleteAccountSubscription.callCount, 0);
assert.equal(log.notifyAttachedServices.callCount, 0);
assert.equal(mailer.sendDownloadSubscriptionEmail.callCount, 0);
});

it('should activate an account', async () => {
await mockSubHubUpdates(log, config, db, mailer).handleSubHubUpdates(
await mockSubHubUpdates(log, config, db).handleSubHubUpdates(
mockMessage({ active: true })
);
// FIXME: figure out what side effect we expect
@@ -88,7 +85,7 @@ describe('subhub updates', () => {
});

assert.equal(log.notifyAttachedServices.callCount, 1);
let args = log.notifyAttachedServices.args[0];
const args = log.notifyAttachedServices.args[0];
assert.lengthOf(args, 3);
assert.equal(args[0], 'subscription:update');
assert.isObject(args[1]);
@@ -102,16 +99,6 @@ describe('subhub updates', () => {
productId: baseMessage.productName,
productCapabilities: ['foo', 'bar'],
});

assert.equal(mailer.sendDownloadSubscriptionEmail.callCount, 1);
args = mailer.sendDownloadSubscriptionEmail.args[0];
assert.lengthOf(args, 3);
assert.isArray(args[0]);
assert.equal(args[1].uid, baseMessage.uid);
assert.deepEqual(args[2], {
acceptLanguage: 'flub',
productId: baseMessage.productName,
});
});

it('should skip activating a subscription that already exists', async () => {
@@ -127,9 +114,7 @@ describe('subhub updates', () => {
createdAt: message.eventCreatedAt - 10000,
};
});
await mockSubHubUpdates(log, config, db, mailer).handleSubHubUpdates(
message
);
await mockSubHubUpdates(log, config, db).handleSubHubUpdates(message);
assert.equal(
db.getAccountSubscription.callCount,
1,
@@ -140,22 +125,10 @@ describe('subhub updates', () => {
0,
'db.createAccountSubscription() should not be called'
);

// The download subscription email should still get sent even if the
// subscription is already activated, though.
assert.equal(mailer.sendDownloadSubscriptionEmail.callCount, 1);
const args = mailer.sendDownloadSubscriptionEmail.args[0];
assert.lengthOf(args, 3);
assert.isArray(args[0]);
assert.equal(args[1].uid, baseMessage.uid);
assert.deepEqual(args[2], {
acceptLanguage: 'flub',
productId: baseMessage.productName,
});
});

it('should de-activate an account', async () => {
await mockSubHubUpdates(log, config, db, mailer).handleSubHubUpdates(
await mockSubHubUpdates(log, config, db).handleSubHubUpdates(
mockMessage({ active: false })
);
// FIXME: figure out what side effect we expect
@@ -197,9 +170,7 @@ describe('subhub updates', () => {
createdAt: message.eventCreatedAt + 1000,
};
});
await mockSubHubUpdates(log, config, db, mailer).handleSubHubUpdates(
message
);
await mockSubHubUpdates(log, config, db).handleSubHubUpdates(message);
assert.equal(
db.getAccountSubscription.callCount,
1,
@@ -228,9 +199,7 @@ describe('subhub updates', () => {
createdAt: message.eventCreatedAt + 1000,
};
});
await mockSubHubUpdates(log, config, db, mailer).handleSubHubUpdates(
message
);
await mockSubHubUpdates(log, config, db).handleSubHubUpdates(message);
assert.equal(
db.getAccountSubscription.callCount,
1,

0 comments on commit 750b54b

Please sign in to comment.
You can’t perform that action at this time.