From 355de5b0a4c0e3a7e7c7d94d6b8343dbc14c837c Mon Sep 17 00:00:00 2001 From: Girish Ramakrishnan Date: Mon, 19 Apr 2021 14:22:22 -0700 Subject: [PATCH] notifications: fix update notification the notification wasn't working because this was in apptask and the apptask died before it could send out the email. we now move the notification to box process and also remove the email notification. --- package-lock.json | 74 +++++++++++++++++++++--------- src/apps.js | 9 ++-- src/apptask.js | 4 +- src/auditsource.js | 1 - src/mail_templates/app_updated.ejs | 40 ---------------- src/mail_templates/test_data.json | 48 ------------------- src/mailer.js | 41 ----------------- src/notifications.js | 12 +---- 8 files changed, 60 insertions(+), 169 deletions(-) delete mode 100644 src/mail_templates/app_updated.ejs delete mode 100644 src/mail_templates/test_data.json diff --git a/package-lock.json b/package-lock.json index bbf007445..900d511b9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -316,9 +316,9 @@ "integrity": "sha1-x57Zf380y48robyXkLzDZkdLS3k=" }, "aws-sdk": { - "version": "2.879.0", - "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.879.0.tgz", - "integrity": "sha512-HRfjGwST1U9AvCJFAyqpAJwbjFR4LqUyEUk77qdJpdYHL9pGPHdnEfGRkBkPn36xcC7Em6gVvFveVoEihbQUyQ==", + "version": "2.888.0", + "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.888.0.tgz", + "integrity": "sha512-9Rg14eneXnrs5Wh5FL42qGEXf7QaqaV/gMHU9SfvAA0SEM390QnwVjCSKF5YAReWjSuJriKJTDiodMI39J+Nrg==", "requires": { "buffer": "4.9.2", "events": "1.1.1", @@ -1070,6 +1070,13 @@ "which-module": "^2.0.0", "y18n": "^4.0.0", "yargs-parser": "^18.1.2" + }, + "dependencies": { + "y18n": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/y18n/-/y18n-4.0.3.tgz", + "integrity": "sha512-JKhqTOwSrqNA1NY5lSztJ1GrBiUodLMmIZuLiDaMRJ+itFd+ABVE8XBjOvIWL+rSqNDC74LCSFmlb/U4UZ4hJQ==" + } } }, "yargs-parser": { @@ -2264,9 +2271,9 @@ "dev": true }, "js-yaml": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.0.0.tgz", - "integrity": "sha512-pqon0s+4ScYUvX30wxQi3PogGFAlUyH0awepWvwkj4jD4v+ova3RiYw8bmA6x2rDrEaj8i/oWKoRxpVNW+Re8Q==", + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", + "integrity": "sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==", "requires": { "argparse": "^2.0.1" } @@ -2730,11 +2737,18 @@ } }, "mkdirp": { - "version": "0.5.1", - "resolved": false, - "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", + "version": "0.5.5", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz", + "integrity": "sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ==", "requires": { - "minimist": "0.0.8" + "minimist": "^1.2.5" + }, + "dependencies": { + "minimist": { + "version": "1.2.5", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", + "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==" + } } }, "mkdirp-classic": { @@ -2844,6 +2858,15 @@ "integrity": "sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==", "dev": true }, + "js-yaml": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.0.0.tgz", + "integrity": "sha512-pqon0s+4ScYUvX30wxQi3PogGFAlUyH0awepWvwkj4jD4v+ova3RiYw8bmA6x2rDrEaj8i/oWKoRxpVNW+Re8Q==", + "dev": true, + "requires": { + "argparse": "^2.0.1" + } + }, "locate-path": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-6.0.0.tgz", @@ -4094,12 +4117,19 @@ "which-module": "^2.0.0", "y18n": "^4.0.0", "yargs-parser": "^15.0.0" + }, + "dependencies": { + "y18n": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/y18n/-/y18n-4.0.3.tgz", + "integrity": "sha512-JKhqTOwSrqNA1NY5lSztJ1GrBiUodLMmIZuLiDaMRJ+itFd+ABVE8XBjOvIWL+rSqNDC74LCSFmlb/U4UZ4hJQ==" + } } }, "yargs-parser": { - "version": "15.0.0", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-15.0.0.tgz", - "integrity": "sha512-xLTUnCMc4JhxrPEPUYD5IBR1mWCK/aT6+RJ/K29JY2y1vD+FhtgKK0AXRWvI262q3QSffAQuTouFIKUuHX89wQ==", + "version": "15.0.1", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-15.0.1.tgz", + "integrity": "sha512-0OAMV2mAZQrs3FkNpDQcBk1x5HXb8X4twADss4S0Iuk+2dGnLOE/fRHrsYm542GduMveyA77OF4wrNJuanRCWw==", "requires": { "camelcase": "^5.0.0", "decamelize": "^1.2.0" @@ -4390,9 +4420,9 @@ "integrity": "sha512-s8+wktIuDSLffCywiwSxQOMqtPxML11a/dtHE17tMn4B1MSWw/C22EKf7M2KGUBcDaVFEGT+S8N02geDXeuNKg==" }, "minimist": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", - "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=" + "version": "1.2.5", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", + "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==" }, "prettyjson": { "version": "1.2.1", @@ -4671,9 +4701,9 @@ } }, "underscore": { - "version": "1.12.1", - "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.12.1.tgz", - "integrity": "sha512-hEQt0+ZLDVUMhebKxL4x1BTtDY7bavVofhZ9KZ4aI26X9SRaE+Y3m83XUL1UP2jn8ynjndwCCpEHdUG+9pP1Tw==" + "version": "1.13.1", + "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.13.1.tgz", + "integrity": "sha512-hzSoAVtJF+3ZtiFX0VgfFPHEDRm7Y/QPjGyNo4TVdnDTdft3tr8hEkD25a1jC+TjTuE7tkHGKkhwCgs9dgBB2g==" }, "unique-string": { "version": "2.0.0", @@ -4969,9 +4999,9 @@ "integrity": "sha512-LKYU1iAXJXUgAXn9URjiu+MWhyUXHsvfp7mcuYm9dSUKK0/CjtrUwFAxD82/mCWbtLsGjFIad0wIsod4zrTAEQ==" }, "y18n": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/y18n/-/y18n-4.0.0.tgz", - "integrity": "sha512-r9S/ZyXu/Xu9q1tYlpsLIsa3EeLXXk0VwlxqTcFRfg9EhMW+17kbt9G0NrgCmhGb5vT2hyhJZLfDGx+7+5Uj/w==" + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/y18n/-/y18n-4.0.3.tgz", + "integrity": "sha512-JKhqTOwSrqNA1NY5lSztJ1GrBiUodLMmIZuLiDaMRJ+itFd+ABVE8XBjOvIWL+rSqNDC74LCSFmlb/U4UZ4hJQ==" }, "yallist": { "version": "3.1.1", diff --git a/src/apps.js b/src/apps.js index 3049ec215..ed4d6ea5d 100644 --- a/src/apps.js +++ b/src/apps.js @@ -649,7 +649,7 @@ function scheduleTask(appId, installationState, taskId, callback) { } else if (!(installationState === exports.ISTATE_PENDING_UNINSTALL && !error)) { // clear out taskId except for successful uninstall appdb.update(appId, { taskId: null }, callback); } else { - callback(); + callback(null); } }); }); @@ -674,7 +674,7 @@ function addTask(appId, installationState, task, callback) { if (error && error.reason === BoxError.NOT_FOUND) return callback(new BoxError(BoxError.BAD_STATE, 'Another task is scheduled for this app')); // could be because app went away OR a taskId exists if (error) return callback(error); - if (scheduleNow) scheduleTask(appId, installationState, taskId, NOOP_CALLBACK); + if (scheduleNow) scheduleTask(appId, installationState, taskId, task.onFinished || NOOP_CALLBACK); callback(null, { taskId }); }); @@ -1374,7 +1374,10 @@ function update(app, data, auditSource, callback) { const task = { args: { updateConfig }, - values + values, + onFinished: (error) => { + eventlog.add(eventlog.ACTION_APP_UPDATE_FINISH, auditSource, { app, success: !error, errorMessage: error ? error.message : null }, () => {}); // ignore error + } }; addTask(appId, exports.ISTATE_PENDING_UPDATE, task, function (error, result) { if (error) return callback(error); diff --git a/src/apptask.js b/src/apptask.js index 1ebca168f..fc904ce90 100644 --- a/src/apptask.js +++ b/src/apptask.js @@ -18,7 +18,6 @@ const appdb = require('./appdb.js'), apps = require('./apps.js'), assert = require('assert'), async = require('async'), - auditSource = require('./auditsource.js'), backups = require('./backups.js'), BoxError = require('./boxerror.js'), collectd = require('./collectd.js'), @@ -28,7 +27,6 @@ const appdb = require('./appdb.js'), docker = require('./docker.js'), domains = require('./domains.js'), ejs = require('ejs'), - eventlog = require('./eventlog.js'), fs = require('fs'), iputils = require('./iputils.js'), manifestFormat = require('cloudron-manifestformat'), @@ -846,7 +844,7 @@ function update(app, args, progressCallback, callback) { debugApp(app, 'Error updating app: %s', error); updateApp(app, { installationState: apps.ISTATE_ERROR, error: makeTaskError(error, app) }, callback.bind(null, error)); } else { - eventlog.add(eventlog.ACTION_APP_UPDATE_FINISH, auditSource.APP_TASK, { app: app, success: true }, () => callback()); // ignore error + callback(null); } }); } diff --git a/src/auditsource.js b/src/auditsource.js index 4d3ea0856..0fd97150d 100644 --- a/src/auditsource.js +++ b/src/auditsource.js @@ -3,7 +3,6 @@ exports = module.exports = { CRON: { userId: null, username: 'cron' }, HEALTH_MONITOR: { userId: null, username: 'healthmonitor' }, - APP_TASK: { userId: null, username: 'apptask' }, EXTERNAL_LDAP_TASK: { userId: null, username: 'externalldap' }, EXTERNAL_LDAP_AUTO_CREATE: { userId: null, username: 'externalldap' }, diff --git a/src/mail_templates/app_updated.ejs b/src/mail_templates/app_updated.ejs deleted file mode 100644 index 563fd6896..000000000 --- a/src/mail_templates/app_updated.ejs +++ /dev/null @@ -1,40 +0,0 @@ -<%if (format === 'text') { %> - -Dear Cloudron Admin, - -The application '<%= title %>' installed at <%= appFqdn %> was updated to app package version <%= version %>. - -Changes: -<%= changelog %> - -Powered by https://cloudron.io - -Sent at: <%= new Date().toUTCString() %> - -<% } else { %> - -
- - - -

Dear <%= cloudronName %> Admin,

- -
- -
- The application '<%= title %>' installed at <%= appFqdn %> was updated to app package version <%= version %>. - -
Changelog:
- <%- changelogHTML %> -
- -
-
- -
- Powered by Cloudron.
- Sent at: <%= new Date().toUTCString() %> -
- -
-<% } %> diff --git a/src/mail_templates/test_data.json b/src/mail_templates/test_data.json deleted file mode 100644 index b760495fb..000000000 --- a/src/mail_templates/test_data.json +++ /dev/null @@ -1,48 +0,0 @@ -{ - "app_updated.ejs": { - "format": "html", - "title": "WordPress", - "appFqdn": "updated.smartserver.io", - "version": "1.3.4", - "changelog": "* This has changed\n * and that as well", - "changelogHTML": "", - "cloudronName": "Smartserver", - "cloudronAvatarUrl": "https://cloudron.io/img/logo.png" - }, - "app_updates_available.ejs": { - "format": "html", - "webadminUrl": "https://my.cloudron.io", - "cloudronName": "Smartserver", - "cloudronAvatarUrl": "https://cloudron.io/img/logo.png", - "hasSubscription": true, - "apps": [{ - "updateInfo": { - "manifest": { - "version": "1.4.3" - } - }, - "app": { - "fqdn": "site.smartserver.io", - "manifest": { - "title": "WordPress" - } - }, - "changelog": "* This has changed\n * and that as well", - "changelogHTML": "" - }, { - "updateInfo": { - "manifest": { - "version": "0.1.3" - } - }, - "app": { - "fqdn": "another.smartserver.io", - "manifest": { - "title": "RocketChat" - } - }, - "changelog": "* This has changed\n * and that as well", - "changelogHTML": "" - }] - } -} diff --git a/src/mailer.js b/src/mailer.js index c02a8c742..bf60871cf 100644 --- a/src/mailer.js +++ b/src/mailer.js @@ -7,8 +7,6 @@ exports = module.exports = { sendInvite, - appUpdated, - backupFailed, certificateRenewalError, @@ -174,45 +172,6 @@ function passwordReset(user) { }); } -function appUpdated(mailTo, app, callback) { - assert.strictEqual(typeof mailTo, 'string'); - assert.strictEqual(typeof app, 'object'); - callback = callback || NOOP_CALLBACK; - - debug('Sending mail for app %s @ %s updated', app.id, app.fqdn); - - getMailConfig(function (error, mailConfig) { - if (error) return debug('Error getting mail details:', error); - - var converter = new showdown.Converter(); - var templateData = { - title: app.manifest.title, - appFqdn: app.fqdn, - version: app.manifest.version, - changelog: app.manifest.changelog, - changelogHTML: converter.makeHtml(app.manifest.changelog), - cloudronName: mailConfig.cloudronName, - cloudronAvatarUrl: settings.adminOrigin() + '/api/v1/cloudron/avatar' - }; - - var templateDataText = JSON.parse(JSON.stringify(templateData)); - templateDataText.format = 'text'; - - var templateDataHTML = JSON.parse(JSON.stringify(templateData)); - templateDataHTML.format = 'html'; - - var mailOptions = { - from: mailConfig.notificationFrom, - to: mailTo, - subject: `[${mailConfig.cloudronName}] App ${app.fqdn} was updated`, - text: render('app_updated.ejs', templateDataText), - html: render('app_updated.ejs', templateDataHTML) - }; - - sendMail(mailOptions, callback); - }); -} - function boxUpdateAvailable(mailTo, updateInfo, callback) { assert.strictEqual(typeof mailTo, 'string'); assert.strictEqual(typeof updateInfo, 'object'); diff --git a/src/notifications.js b/src/notifications.js index e6687905a..ed3ebc2e8 100644 --- a/src/notifications.js +++ b/src/notifications.js @@ -44,8 +44,6 @@ function add(userId, eventId, title, message, callback) { assert.strictEqual(typeof message, 'string'); assert.strictEqual(typeof callback, 'function'); - debug('add: ', userId, title); - notificationdb.add({ userId: userId, eventId: eventId, @@ -203,14 +201,7 @@ function appUpdated(eventId, app, callback) { : `${app.manifest.title} at ${app.fqdn} updated to package version ${app.manifest.version}`; forEachAdmin({ skip: [] }, function (admin, done) { - add(admin.id, eventId, title, `The application installed at https://${app.fqdn} was updated.\n\nChangelog:\n${app.manifest.changelog}\n`, function (error) { - if (error) return callback(error); - - mailer.appUpdated(admin.email, app, function (error) { - if (error) debug('appUpdated: Failed to send app updated email', error); // non fatal - done(); - }); - }); + add(admin.id, eventId, title, `The application installed at https://${app.fqdn} was updated.\n\nChangelog:\n${app.manifest.changelog}\n`, done); }, callback); } @@ -303,7 +294,6 @@ function alert(id, title, message, callback) { assert.strictEqual(typeof callback, 'function'); const acknowledged = !message; - debug(`alert: id=${id} title=${title} ack=${acknowledged}`); forEachAdmin({ skip: [] }, function (admin, callback) { const data = {