From 645d647248eaf8e15fbd2cd96da0a45fe7a2fa9f Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Sat, 2 Nov 2019 13:11:02 -0400 Subject: [PATCH] feat: wip, better digest handling (+ eventual digest resend logic) (#7995) * feat: wip, better digest handling (+ eventual digest resend logic) - await emailer.send call in digest.send method - save send success to a new sorted set digest:{interval}:byUid * feat: continuing work on digest tools - Added ACP page to view digest settings and delivery times per user * feat: added paginator and stub buttons for resending digest * feat: wrapping up digest revamp - New language strings in ACP digest page - Client-side ACP script for digest ACP page - Websocket call for ACP page to execute digests - Broke out logic to retrieve user digest settings to getUsersInterval * fix: minor cleanup * fix: #8010 and some style suggestions from baris * fix: resolve confusing comment --- .tx/config | 50 ++++++++++ .../language/en-GB/admin/manage/digest.json | 21 ++++ public/language/en-GB/admin/menu.json | 1 + public/language/en-GB/error.json | 1 + public/src/admin/manage/digest.js | 45 +++++++++ src/controllers/admin.js | 1 + src/controllers/admin/digest.js | 23 +++++ src/routes/admin.js | 1 + src/socket.io/admin.js | 19 ++++ src/user/digest.js | 97 ++++++++++++++++--- src/views/admin/manage/digest.tpl | 51 ++++++++++ src/views/admin/partials/menu.tpl | 2 + 12 files changed, 296 insertions(+), 16 deletions(-) create mode 100644 public/language/en-GB/admin/manage/digest.json create mode 100644 public/src/admin/manage/digest.js create mode 100644 src/controllers/admin/digest.js create mode 100644 src/views/admin/manage/digest.tpl diff --git a/.tx/config b/.tx/config index a847b449e0..be0c9bbcd8 100644 --- a/.tx/config +++ b/.tx/config @@ -2650,6 +2650,56 @@ trans.zh_CN = public/language/zh-CN/admin/manage/users.json trans.zh_TW = public/language/zh-TW/admin/manage/users.json type = KEYVALUEJSON +[nodebb.admin-manage-digest] +file_filter = public/language//admin/manage/digest.json +source_file = public/language/en-GB/admin/manage/digest.json +source_lang = en_GB +trans.ar = public/language/ar/admin/manage/digest.json +trans.bg = public/language/bg/admin/manage/digest.json +trans.bn = public/language/bn/admin/manage/digest.json +trans.cs = public/language/cs/admin/manage/digest.json +trans.da = public/language/da/admin/manage/digest.json +trans.de = public/language/de/admin/manage/digest.json +trans.el = public/language/el/admin/manage/digest.json +trans.en@pirate = public/language/en-x-pirate/admin/manage/digest.json +trans.en_US = public/language/en-US/admin/manage/digest.json +trans.es = public/language/es/admin/manage/digest.json +trans.et = public/language/et/admin/manage/digest.json +trans.fa_IR = public/language/fa-IR/admin/manage/digest.json +trans.fi = public/language/fi/admin/manage/digest.json +trans.fr = public/language/fr/admin/manage/digest.json +trans.gl = public/language/gl/admin/manage/digest.json +trans.he = public/language/he/admin/manage/digest.json +trans.hr = public/language/hr/admin/manage/digest.json +trans.hu = public/language/hu/admin/manage/digest.json +trans.id = public/language/id/admin/manage/digest.json +trans.it = public/language/it/admin/manage/digest.json +trans.ja = public/language/ja/admin/manage/digest.json +trans.ko = public/language/ko/admin/manage/digest.json +trans.lt = public/language/lt/admin/manage/digest.json +trans.lv = public/language/lv/admin/manage/digest.json +trans.ms = public/language/ms/admin/manage/digest.json +trans.nb = public/language/nb/admin/manage/digest.json +trans.nl = public/language/nl/admin/manage/digest.json +trans.pl = public/language/pl/admin/manage/digest.json +trans.pt_BR = public/language/pt-BR/admin/manage/digest.json +trans.pt_PT = public/language/pt-PT/admin/manage/digest.json +trans.ro = public/language/ro/admin/manage/digest.json +trans.ru = public/language/ru/admin/manage/digest.json +trans.rw = public/language/rw/admin/manage/digest.json +trans.sc = public/language/sc/admin/manage/digest.json +trans.sk = public/language/sk/admin/manage/digest.json +trans.sl = public/language/sl/admin/manage/digest.json +trans.sr = public/language/sr/admin/manage/digest.json +trans.sv = public/language/sv/admin/manage/digest.json +trans.th = public/language/th/admin/manage/digest.json +trans.tr = public/language/tr/admin/manage/digest.json +trans.uk = public/language/uk/admin/manage/digest.json +trans.vi = public/language/vi/admin/manage/digest.json +trans.zh_CN = public/language/zh-CN/admin/manage/digest.json +trans.zh_TW = public/language/zh-TW/admin/manage/digest.json +type = KEYVALUEJSON + [nodebb.admin-settings-advanced] file_filter = public/language//admin/settings/advanced.json source_file = public/language/en-GB/admin/settings/advanced.json diff --git a/public/language/en-GB/admin/manage/digest.json b/public/language/en-GB/admin/manage/digest.json new file mode 100644 index 0000000000..075b8e1aff --- /dev/null +++ b/public/language/en-GB/admin/manage/digest.json @@ -0,0 +1,21 @@ +{ + "lead": "A listing of digest delivery stats and times is displayed below.", + "disclaimer": "Please be advised that email delivery is not guaranteed, due to the nature of email technology. Many variables factor into whether an email sent to the recipient server is ultimately delivered into the user's inbox, including server reputation, blacklisted IP addresses, and whether DKIM/SPF/DMARC is configured.", + "disclaimer-continued": "A successful delivery means the message was sent successfully by NodeBB and acknowledged by the recipient server. It does not mean the email landed in the inbox. For best results, we recommend using a third-party email delivery service such as SendGrid.", + + "user": "User", + "subscription": "Subscription Type", + "last-delivery": "Last successful delivery", + "default": "System default", + "default-help": "System default means the user has not explicitly overridden the global forum setting for digests, which is currently: "%1"", + "resend": "Resend Digest", + "resend-all-confirm": "Are you sure you wish to mnually execute this digest run?", + "resent-single": "Manual digest resend completed", + "resent-day": "Daily digest resent", + "resent-week": "Weekly digest resent", + "resent-month": "Monthly digest resent", + "null": "Never", + "manual-run": "Manual digest run:", + + "no-delivery-data": "No delivery data found" +} \ No newline at end of file diff --git a/public/language/en-GB/admin/menu.json b/public/language/en-GB/admin/menu.json index cd95b5891f..1f275e03ae 100644 --- a/public/language/en-GB/admin/menu.json +++ b/public/language/en-GB/admin/menu.json @@ -18,6 +18,7 @@ "manage/groups": "Groups", "manage/ip-blacklist": "IP Blacklist", "manage/uploads": "Uploads", + "manage/digest": "Digests", "section-settings": "Settings", "settings/general": "General", diff --git a/public/language/en-GB/error.json b/public/language/en-GB/error.json index 55dde13268..e2fab65924 100644 --- a/public/language/en-GB/error.json +++ b/public/language/en-GB/error.json @@ -38,6 +38,7 @@ "email-confirm-failed": "We could not confirm your email, please try again later.", "confirm-email-already-sent": "Confirmation email already sent, please wait %1 minute(s) to send another one.", "sendmail-not-found": "The sendmail executable could not be found, please ensure it is installed and executable by the user running NodeBB.", + "digest-not-enabled": "This user does not have digests enabled, or the system default is not configured to send digests", "username-too-short": "Username too short", "username-too-long": "Username too long", diff --git a/public/src/admin/manage/digest.js b/public/src/admin/manage/digest.js new file mode 100644 index 0000000000..3b07918277 --- /dev/null +++ b/public/src/admin/manage/digest.js @@ -0,0 +1,45 @@ +'use strict'; + + +define('admin/manage/digest', function () { + var Digest = {}; + + Digest.init = function () { + $('table').on('click', '[data-action]', function () { + var action = this.getAttribute('data-action'); + var uid = this.getAttribute('data-uid'); + + if (action.startsWith('resend-')) { + var interval = action.slice(7); + bootbox.confirm('[[admin/manage/digest:resend-all-confirm]]', function (ok) { + if (ok) { + Digest.send(action, undefined, function (err) { + if (err) { + return app.alertError(err); + } + + app.alertSuccess('[[admin/manage/digest:resent-' + interval + ']]'); + }); + } + }); + } else { + Digest.send(action, uid, function (err) { + if (err) { + return app.alertError(err); + } + + app.alertSuccess('[[admin/manage/digest:resent-single]]'); + }); + } + }); + }; + + Digest.send = function (action, uid, callback) { + socket.emit('admin.digest.resend', { + action: action, + uid: uid, + }, callback); + }; + + return Digest; +}); diff --git a/src/controllers/admin.js b/src/controllers/admin.js index 019192fd3c..267e1f9e6d 100644 --- a/src/controllers/admin.js +++ b/src/controllers/admin.js @@ -9,6 +9,7 @@ var adminController = { postQueue: require('./admin/postqueue'), blacklist: require('./admin/blacklist'), groups: require('./admin/groups'), + digest: require('./admin/digest'), appearance: require('./admin/appearance'), extend: { widgets: require('./admin/widgets'), diff --git a/src/controllers/admin/digest.js b/src/controllers/admin/digest.js new file mode 100644 index 0000000000..48eddcddcf --- /dev/null +++ b/src/controllers/admin/digest.js @@ -0,0 +1,23 @@ +'use strict'; + +const meta = require('../../meta'); +const digest = require('../../user/digest'); +const pagination = require('../../pagination'); + +const digestController = module.exports; + +digestController.get = async function (req, res) { + const page = parseInt(req.query.page, 10) || 1; + const resultsPerPage = 50; + const start = Math.max(0, page - 1) * resultsPerPage; + const stop = start + resultsPerPage - 1; + const delivery = await digest.getDeliveryTimes(start, stop); + + const pageCount = Math.ceil(delivery.count / resultsPerPage); + res.render('admin/manage/digest', { + title: '[[admin/menu:manage/digest]]', + delivery: delivery.users, + default: meta.config.dailyDigestFreq, + pagination: pagination.create(page, pageCount), + }); +}; diff --git a/src/routes/admin.js b/src/routes/admin.js index f6a92eac61..da9fbc0b4c 100644 --- a/src/routes/admin.js +++ b/src/routes/admin.js @@ -80,6 +80,7 @@ function addRoutes(router, middleware, controllers) { router.get('/manage/groups/:name', middlewares, controllers.admin.groups.get); router.get('/manage/uploads', middlewares, controllers.admin.uploads.get); + router.get('/manage/digest', middlewares, controllers.admin.digest.get); router.get('/settings/:term?', middlewares, controllers.admin.settings.get); diff --git a/src/socket.io/admin.js b/src/socket.io/admin.js index 15791388ea..4c6a748be8 100644 --- a/src/socket.io/admin.js +++ b/src/socket.io/admin.js @@ -42,6 +42,7 @@ SocketAdmin.analytics = {}; SocketAdmin.logs = {}; SocketAdmin.errors = {}; SocketAdmin.uploads = {}; +SocketAdmin.digest = {}; SocketAdmin.before = async function (socket, method) { const isAdmin = await user.isAdministrator(socket.uid); @@ -352,4 +353,22 @@ SocketAdmin.uploads.delete = function (socket, pathToFile, callback) { fs.unlink(pathToFile, callback); }; +SocketAdmin.digest.resend = async (socket, data) => { + const uid = data.uid; + const interval = data.action.startsWith('resend-') ? data.action.slice(7) : await userDigest.getUsersInterval(uid); + + if (!interval && meta.config.dailyDigestFreq === 'off') { + throw new Error('[[error:digest-not-enabled]]'); + } + + if (uid) { + await userDigest.execute({ + interval: interval || meta.config.dailyDigestFreq, + subscribers: [uid], + }); + } else { + await userDigest.execute({ interval: interval }); + } +}; + require('../promisify')(SocketAdmin); diff --git a/src/user/digest.js b/src/user/digest.js index e35db90b49..6ada20c9c1 100644 --- a/src/user/digest.js +++ b/src/user/digest.js @@ -4,6 +4,7 @@ const async = require('async'); const winston = require('winston'); const nconf = require('nconf'); +const db = require('../database'); const batch = require('../batch'); const meta = require('../meta'); const user = require('../user'); @@ -28,17 +29,49 @@ Digest.execute = async function (payload) { return; } try { - const count = await Digest.send({ + await Digest.send({ interval: payload.interval, subscribers: subscribers, }); - winston.info('[user/jobs] Digest (' + payload.interval + ') scheduling completed. ' + count + ' email(s) sent.'); + winston.info('[user/jobs] Digest (' + payload.interval + ') scheduling completed. Sending emails; this may take some time...'); } catch (err) { winston.error('[user/jobs] Could not send digests (' + payload.interval + ')', err); throw err; } }; +Digest.getUsersInterval = async (uids) => { + // Checks whether user specifies digest setting, or null/false for system default setting + let single = false; + if (!Array.isArray(uids) && !isNaN(parseInt(uids, 10))) { + uids = [uids]; + single = true; + } + + let settings = await Promise.all([ + db.isSortedSetMembers('digest:day:uids', uids), + db.isSortedSetMembers('digest:week:uids', uids), + db.isSortedSetMembers('digest:month:uids', uids), + ]); + settings = settings.reduce((memo, cur, idx) => { + switch (idx) { + case 0: + memo = cur.map(bool => (bool === true ? 'day' : bool)); + break; + case 1: + memo = cur.map(bool => (bool === true ? 'week' : bool)); + break; + case 2: + memo = cur.map(bool => (bool === true ? 'month' : bool)); + break; + } + + return memo; + }); + + return single ? settings[0] : settings; +}; + Digest.getSubscribers = async function (interval) { var subscribers = []; @@ -99,21 +132,53 @@ Digest.send = async function (data) { return topicObj; }); emailsSent += 1; - emailer.send('digest', userObj.uid, { - subject: '[[email:digest.subject, ' + (now.getFullYear() + '/' + (now.getMonth() + 1) + '/' + now.getDate()) + ']]', - username: userObj.username, - userslug: userObj.userslug, - notifications: notifications, - recent: topicsData, - interval: data.interval, - showUnsubscribe: true, - }, function (err) { - if (err) { - winston.error('[user/jobs] Could not send digest email', err); - } - }); + try { + await emailer.send('digest', userObj.uid, { + subject: '[[email:digest.subject, ' + (now.getFullYear() + '/' + (now.getMonth() + 1) + '/' + now.getDate()) + ']]', + username: userObj.username, + userslug: userObj.userslug, + notifications: notifications, + recent: topicsData, + interval: data.interval, + showUnsubscribe: true, + }); + } catch (err) { + winston.error('[user/jobs] Could not send digest email', err); + } + + if (data.interval !== 'alltime') { + await db.sortedSetAdd('digest:delivery', now.getTime(), userObj.uid); + } + }, function () { + winston.info('[user/jobs] Digest (' + data.interval + ') sending completed. ' + emailsSent + ' emails sent.'); }); - return emailsSent; +}; + +Digest.getDeliveryTimes = async (start, stop) => { + const count = await db.sortedSetCard('users:joindate'); + const uids = await user.getUidsFromSet('users:joindate', start, stop); + if (!uids) { + return []; + } + + // Grab the last time a digest was successfully delivered to these uids + const scores = await db.sortedSetScores('digest:delivery', uids); + + // Get users' digest settings + const settings = await Digest.getUsersInterval(uids); + + // Populate user data + let userData = await user.getUsersFields(uids, ['username', 'picture']); + userData = userData.map((user, idx) => { + user.lastDelivery = scores[idx] ? new Date(scores[idx]).toISOString() : '[[admin/manage/digest:null]]'; + user.setting = settings[idx]; + return user; + }); + + return { + users: userData, + count: count, + }; }; async function getTermTopics(term, uid, start, stop) { diff --git a/src/views/admin/manage/digest.tpl b/src/views/admin/manage/digest.tpl new file mode 100644 index 0000000000..185e89d02b --- /dev/null +++ b/src/views/admin/manage/digest.tpl @@ -0,0 +1,51 @@ +

[[admin/manage/digest:lead]]

+

[[admin/manage/digest:disclaimer]]

+

[[admin/manage/digest:disclaimer-continued]]

+ +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
[[admin/manage/digest:user]][[admin/manage/digest:subscription]][[admin/manage/digest:last-delivery]]
{buildAvatar(delivery, "sm", true)} {../username}{{{if ../setting}}}{../setting}{{{else}}}[[admin/manage/digest:default]]{{{end}}}{../lastDelivery}
+
+ [[admin/manage/digest:no-delivery-data]] +
+
+ [[admin/manage/digest:default-help, {default}]] +
+ [[admin/manage/digest:manual-run]] + + + +
diff --git a/src/views/admin/partials/menu.tpl b/src/views/admin/partials/menu.tpl index fc8cf398a7..06b7fdea43 100644 --- a/src/views/admin/partials/menu.tpl +++ b/src/views/admin/partials/menu.tpl @@ -34,6 +34,7 @@
  • [[admin/menu:manage/post-queue]]
  • [[admin/menu:manage/ip-blacklist]]
  • [[admin/menu:manage/uploads]]
    • +
  • [[admin/menu:manage/digest]]
    • @@ -195,6 +196,7 @@
  • [[admin/menu:manage/post-queue]]
  • [[admin/menu:manage/ip-blacklist]]
  • [[admin/menu:manage/uploads]]
    • +
  • [[admin/menu:manage/digest]]