Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated notification support #131

Closed
wants to merge 13 commits into from
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ services:
NODE_ENV: development
PORT: 8100
REDIS_DSN: redis://redis:6379/0
SENDGRID_API_KEY: ${SENDGRID_API_KEY}
SENDGRID_FROM: ${SENDGRID_FROM}
redis:
image: redis:alpine
mongo:
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"body-parser": "^1.18.2",
"cors": "^2.8.4",
"crawler-user-agents": "https://github.com/monperrus/crawler-user-agents.git#master",
"cron": "^1.3.0",
"deep-assign": "^2.0.0",
"deepmerge": "^2.1.0",
"elasticsearch": "^15.0.0",
Expand Down
3 changes: 3 additions & 0 deletions src/email-templates/campaign.created.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<p>Campaign "{{ campaign.name }}" was created.</p>

<p>Click here to submit your creatives: <a href="{{ materialCollectUri }}">{{ materialCollectUri }}</a></p>
6 changes: 6 additions & 0 deletions src/email-templates/campaign.ended.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<p>Campaign "{{ campaign.name }}" has ended.</p>

<p>Click here to view the summary report: <a href="{{ reportSummaryUri }}">{{ reportSummaryUri }}</a></p>

<p>Click here to view the creative report: <a href="{{ reportCreativeUri }}">{{ reportCreativeUri }}</a></p>

1 change: 1 addition & 0 deletions src/email-templates/campaign.started.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>Campaign "{{ campaign.name }}" has started.</p>
3 changes: 0 additions & 3 deletions src/email-templates/external/campaign.created.hbs

This file was deleted.

3 changes: 0 additions & 3 deletions src/email-templates/internal/campaign.created.hbs

This file was deleted.

56 changes: 42 additions & 14 deletions src/graph/resolvers/campaign.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ const getNotifyDefaults = async (advertiserId, user) => {
return notify;
};

const updateNotifications = async (campaignId) => {
contactNotifier.scheduleCampaignCreated({ campaignId });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updateNotifications isn't awaiting the scheduled email calls, is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

contactNotifier.scheduleCampaignStarted({ campaignId });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, have a look at the each function within the async package. These are probably a good candidates for running in parallel.
https://caolan.github.io/async/docs.html#each

contactNotifier.scheduleCampaignEnded({ campaignId });
};

module.exports = {
/**
*
Expand Down Expand Up @@ -111,26 +117,29 @@ module.exports = {
payload.criteria = { start: payload.startDate };
payload.notify = await getNotifyDefaults(payload.advertiserId, auth.user);
const campaign = await CampaignRepo.create(payload);
contactNotifier.sendInternalCampaignCreated({ campaign });
contactNotifier.sendExternalCampaignCreated({ campaign });
await updateNotifications(campaign.id);
return campaign;
},

/**
*
*/
updateCampaign: (root, { input }, { auth }) => {
updateCampaign: async (root, { input }, { auth }) => {
auth.check();
const { id, payload } = input;
return CampaignRepo.update(id, payload);
const campaign = await CampaignRepo.update(id, payload);
await updateNotifications(id);
return campaign;
},

assignCampaignValue: async (root, { input }) => {
const { id, field, value } = input;
const campaign = await Campaign.findById(id);
if (!campaign) throw new Error(`Unable to assign field '${field}' to campaign: no record found for id '${id}'`);
campaign.set(field, value);
return campaign.save();
await campaign.save();
await updateNotifications(id);
return campaign;
},

/**
Expand All @@ -139,7 +148,9 @@ module.exports = {
campaignCriteria: async (root, { input }, { auth }) => {
auth.check();
const { campaignId, payload } = input;
return CriteriaRepo.setFor(campaignId, payload);
const criteria = await CriteriaRepo.setFor(campaignId, payload);
await updateNotifications(campaignId);
return criteria;
},

campaignUrl: async (root, { input }, { auth }) => {
Expand All @@ -148,16 +159,20 @@ module.exports = {
const campaign = await Campaign.findById(campaignId);
if (!campaign) throw new Error(`Unable to set campaign URL: no campaign found for '${campaignId}'`);
campaign.url = url;
return campaign.save();
await campaign.save();
await updateNotifications(campaignId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does updateNotifications need to be called on every mutation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO yes, as any campaign field could be used within the email message itself, and changes to criteria would necessitate changes to delivery

return campaign;
},

/**
*
*/
addCampaignCreative: (root, { input }, { auth }) => {
addCampaignCreative: async (root, { input }, { auth }) => {
const { campaignId, payload } = input;
auth.checkCampaignAccess(campaignId);
return CreativeRepo.createFor(campaignId, payload);
const creative = await CreativeRepo.createFor(campaignId, payload);
await updateNotifications(campaignId);
return creative;
},

/**
Expand All @@ -167,6 +182,7 @@ module.exports = {
const { campaignId, creativeId } = input;
auth.checkCampaignAccess(campaignId);
await CreativeRepo.removeFrom(campaignId, creativeId);
await updateNotifications(campaignId);
return 'ok';
},

Expand All @@ -176,7 +192,9 @@ module.exports = {
campaignCreativeStatus: async (root, { input }, { auth }) => {
auth.check();
const { campaignId, creativeId, status } = input;
return CreativeRepo.setStatusFor(campaignId, creativeId, status);
const saved = await CreativeRepo.setStatusFor(campaignId, creativeId, status);
await updateNotifications(campaignId);
return saved;
},

/**
Expand All @@ -186,7 +204,13 @@ module.exports = {
const { campaignId, creativeId, payload } = input;
auth.checkCampaignAccess(campaignId);
const { title, teaser, status } = payload;
return CreativeRepo.updateDetailsFor(campaignId, creativeId, { title, teaser, status });
const details = await CreativeRepo.updateDetailsFor(
campaignId,
creativeId,
{ title, teaser, status },
);
await updateNotifications(campaignId);
return details;
},

/**
Expand All @@ -195,16 +219,20 @@ module.exports = {
campaignCreativeImage: async (root, { input }, { auth }) => {
const { campaignId, creativeId, imageId } = input;
auth.checkCampaignAccess(campaignId);
return CreativeRepo.updateImageFor(campaignId, creativeId, imageId);
const image = await CreativeRepo.updateImageFor(campaignId, creativeId, imageId);
await updateNotifications(campaignId);
return image;
},

/**
*
*/
campaignContacts: (root, { input }, { auth }) => {
campaignContacts: async (root, { input }, { auth }) => {
auth.check();
const { id, type, contactIds } = input;
return ContactRepo.setContactsFor(Campaign, id, type, contactIds);
const contacts = await ContactRepo.setContactsFor(Campaign, id, type, contactIds);
await updateNotifications(id);
return contacts;
},
},
};
2 changes: 0 additions & 2 deletions src/handlebars.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,4 @@ handlebars.registerHelper('build-ua-beacon', (context) => {
return new handlebars.SafeString(`<script>if (window.ga) { ga('send', 'event', 'Fortnight', 'load', '${pid || ''}', '${cid || ''}'); }</script>`);
});

handlebars.registerHelper('link-to', (...parts) => parts.filter(el => typeof el === 'string').join('/'));

module.exports = handlebars;
3 changes: 3 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ const env = require('./env');
const output = require('./output');
const pkg = require('../package.json');
const { app } = require('./server');
const contactNotifier = require('./services/contact-notifier');

const { PORT } = env;

const server = app.listen(PORT);
output.write(`🕸️ 🕸️ 🕸️ Express app '${pkg.name}' listening on port ${PORT}`);

contactNotifier.init();

module.exports = server;
4 changes: 4 additions & 0 deletions src/models/campaign-notification.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const mongoose = require('../connections/mongoose/instance');
const schema = require('../schema/campaign-notification');

module.exports = mongoose.model('campaign-notification', schema);
7 changes: 7 additions & 0 deletions src/schema/advertiser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const notifyPlugin = require('../plugins/notify');
const { applyElasticPlugin, setEntityFields } = require('../elastic/mongoose');
const imagePlugin = require('../plugins/image');
const pushIdPlugin = require('../plugins/push-id');
const accountService = require('../services/account');

const schema = new Schema({
name: {
Expand Down Expand Up @@ -38,6 +39,12 @@ schema.pre('save', async function updateCampaigns() {
}
});

schema.virtual('portalUri').get(async function getPortalUri() {
const account = await accountService.retrieve();
const uri = await account.get('uri');
return `${uri}/app/${this.pushId}`;
});

schema.plugin(notifyPlugin);

setEntityFields(schema, 'name');
Expand Down
44 changes: 44 additions & 0 deletions src/schema/campaign-notification.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
const { Schema } = require('mongoose');
const connection = require('../connections/mongoose/instance');

const schema = new Schema({
campaignId: {
type: Schema.Types.ObjectId,
required: true,
validate: {
async validator(v) {
const doc = await connection.model('campaign').findOne({ _id: v }, { _id: 1 });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to .findById(v, { _id: 1 })
My bad for not doing that before you Cmd C+V

if (doc) return true;
return false;
},
message: 'No campaign found for ID {VALUE}',
},
},
type: {
type: String,
enum: ['Campaign Created', 'Campaign Started', 'Campaign Ended'],
required: true,
},
status: {
type: String,
enum: ['Pending', 'Sending', 'Sent', 'Errored'],
default: 'Pending',
},
error: String,
sendAt: {
type: Date,
required: true,
default: () => new Date(),
},

to: [String],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least one two value is required, correct?
Also, can we add validation to these fields?

cc: [String],
bcc: [String],

subject: String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should subject, text, and html all be required? Well, maybe not text

text: String,
html: String,
}, { timestamps: true });


module.exports = schema;
21 changes: 21 additions & 0 deletions src/schema/campaign/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,27 @@ schema.pre('save', async function setAdvertiserName() {
}
});

schema.virtual('portalUri').get(async function getPortalUri() {
const advertiser = await connection.model('advertiser').findOne({ _id: this.advertiserId }, { pushId: 1 });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.findById please :)

const uri = await advertiser.get('portalUri');
return `${uri}/campaigns/${this.pushId}`;
});

schema.virtual('vMaterialCollectUri').get(async function getVMCU() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consolidate all these virtuals to a subdoc? It's getting pretty dirty at the root :)

And why the v prefix? Virtual?

const uri = await this.get('portalUri');
return `${uri}/materials`;
});

schema.virtual('vReportSummaryUri').get(async function getVRSU() {
const uri = await this.get('portalUri');
return `${uri}/report/summary`;
});

schema.virtual('vReportCreativeUri').get(async function getVRCU() {
const uri = await this.get('portalUri');
return `${uri}/report/creative-breakdown`;
});

schema.plugin(notifyPlugin);
schema.plugin(pushIdPlugin, { required: true });

Expand Down
Loading