From c2b1854df631354a977d737e8a00ddbc6fa8889f Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Fri, 8 May 2026 09:57:31 +0200 Subject: [PATCH] fix(admin): test email always sends, regardless of update availability (#418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Send Test Email" button on the Update Notifications settings page called sendUpdateNotificationNow() — which bailed out with "No updates available" when the instance was already on the latest version. Admins on a current install had no way to verify their SMTP / recipient list was working until an update happened to be pending. Reported in #418 by @Rekoo-PS. Changes: - Add migration 087: insert a dedicated `version_update_test` email template (EN + DE, matching the existing version_update_available convention) with copy that reads as a config-check rather than as a real update notice. Subject prefixed with [TEST] so it's unambiguous in the inbox. Variables: current_version, channel, recipient_email. - Replace sendUpdateNotificationNow() with sendTestUpdateNotification() in updateNotificationService.js. The new path: - Always sends — no updateAvailable bail-out. - Uses the version_update_test template. - Falls back gracefully if checkForUpdates fails (so a transient GitHub API hiccup doesn't block a config-check email). - Does NOT update last_notified_version — that field stays owned by the real-update path so a test send doesn't shadow a future genuine notification for the same version. - Wire /admin/system/updates/notifications/send to the renamed function. No frontend change needed (the button already calls this endpoint). Verified locally with the dev mailhog: clicking Send Test Email on a 3.42.3-beta.0 instance (which has no pending update) delivers 4 emails to all admin recipients with subject "[TEST] PicPeak Update Notification — configuration check" and body interpolated correctly. Returns {success: true, successCount: 4, ...} — previously would have returned {success: false, message: "No updates available"}. --- ...7_add_update_notification_test_template.js | 130 ++++++++++++++++++ backend/src/routes/adminSystem.js | 13 +- .../src/services/updateNotificationService.js | 65 +++++---- 3 files changed, 175 insertions(+), 33 deletions(-) create mode 100644 backend/migrations/core/087_add_update_notification_test_template.js diff --git a/backend/migrations/core/087_add_update_notification_test_template.js b/backend/migrations/core/087_add_update_notification_test_template.js new file mode 100644 index 00000000..da337b45 --- /dev/null +++ b/backend/migrations/core/087_add_update_notification_test_template.js @@ -0,0 +1,130 @@ +/** + * Migration 087: Add a dedicated email template for the admin "Send Test + * Email" button on the Update Notifications settings page (#418). + * + * Previously the button reused the version_update_available template via + * sendUpdateNotificationNow(), which bailed out early when no real update + * was pending — so admins on the latest version had no way to verify + * their SMTP / recipient list config worked. + * + * The test template makes the intent unambiguous in the inbox ("This is + * a test of your update-notification setup, no action needed") and lets + * the send code run unconditionally regardless of update availability. + * + * Languages: EN + DE only, matching the convention of the existing + * version_update_available template (070). The email_templates table + * doesn't have nl/pt/ru columns; sendTemplateEmail falls back to EN. + */ + +exports.up = async function(knex) { + console.log('Running migration: 087_add_update_notification_test_template'); + + const existing = await knex('email_templates') + .where('template_key', 'version_update_test') + .first(); + + if (existing) { + console.log(' version_update_test template already exists, skipping insert'); + return; + } + + await knex('email_templates').insert({ + template_key: 'version_update_test', + subject_en: '[TEST] PicPeak Update Notification — configuration check', + subject_de: '[TEST] PicPeak Update-Benachrichtigung — Konfigurationsprüfung', + body_html_en: ` +

This is a test email

+ +

You are receiving this message because an administrator clicked +Send Test Email on the Update Notifications page of your +PicPeak installation.

+ +
+

Installed version: {{current_version}}

+

Channel: {{channel}}

+

Recipient address: {{recipient_email}}

+
+ +

If you can read this email, your SMTP configuration and the recipient +list are working correctly. When a real new version becomes available, +PicPeak will send a separate notification with release notes and update +instructions.

+ +

No action is required. +You may safely delete this message.

+ +

Best regards,
+Your PicPeak Installation

`, + body_text_en: `This is a test email + +You are receiving this message because an administrator clicked +"Send Test Email" on the Update Notifications page of your PicPeak +installation. + +Installed version: {{current_version}} +Channel: {{channel}} +Recipient address: {{recipient_email}} + +If you can read this email, your SMTP configuration and the recipient +list are working correctly. When a real new version becomes available, +PicPeak will send a separate notification with release notes and update +instructions. + +No action is required. You may safely delete this message. + +Best regards, +Your PicPeak Installation`, + body_html_de: ` +

Dies ist eine Test-E-Mail

+ +

Sie erhalten diese Nachricht, weil ein Administrator auf der Seite +"Update-Benachrichtigungen" Ihrer PicPeak-Installation auf +Test-E-Mail senden geklickt hat.

+ +
+

Installierte Version: {{current_version}}

+

Kanal: {{channel}}

+

Empfänger-Adresse: {{recipient_email}}

+
+ +

Wenn Sie diese E-Mail lesen können, funktionieren Ihre SMTP-Konfiguration +und die Empfängerliste korrekt. Sobald eine echte neue Version verfügbar +ist, sendet PicPeak eine separate Benachrichtigung mit Versionshinweisen +und Update-Anweisungen.

+ +

Es ist keine Aktion +erforderlich. Sie können diese Nachricht gefahrlos löschen.

+ +

Mit freundlichen Grüßen,
+Ihre PicPeak-Installation

`, + body_text_de: `Dies ist eine Test-E-Mail + +Sie erhalten diese Nachricht, weil ein Administrator auf der Seite +"Update-Benachrichtigungen" Ihrer PicPeak-Installation auf +"Test-E-Mail senden" geklickt hat. + +Installierte Version: {{current_version}} +Kanal: {{channel}} +Empfänger-Adresse: {{recipient_email}} + +Wenn Sie diese E-Mail lesen können, funktionieren Ihre SMTP-Konfiguration +und die Empfängerliste korrekt. Sobald eine echte neue Version verfügbar +ist, sendet PicPeak eine separate Benachrichtigung mit Versionshinweisen +und Update-Anweisungen. + +Es ist keine Aktion erforderlich. Sie können diese Nachricht gefahrlos löschen. + +Mit freundlichen Grüßen, +Ihre PicPeak-Installation`, + variables: JSON.stringify(['current_version', 'channel', 'recipient_email']) + }); + + console.log('Migration 087_add_update_notification_test_template completed'); +}; + +exports.down = async function(knex) { + console.log('Rollback: 087_add_update_notification_test_template'); + await knex('email_templates') + .where('template_key', 'version_update_test') + .del(); +}; diff --git a/backend/src/routes/adminSystem.js b/backend/src/routes/adminSystem.js index e44d32a0..e72af785 100644 --- a/backend/src/routes/adminSystem.js +++ b/backend/src/routes/adminSystem.js @@ -11,7 +11,7 @@ const { checkForUpdates, getCurrentChannel } = require('../services/updateCheckS const { detectEnvironment, generateUpdateInstructions } = require('../services/environmentService'); const { checkAndNotifyUpdates, - sendUpdateNotificationNow, + sendTestUpdateNotification, getUpdateNotificationSettings } = require('../services/updateNotificationService'); const router = express.Router(); @@ -352,13 +352,18 @@ router.put('/updates/notifications', adminAuth, requirePermission('settings.edit }); // Manually trigger update notification email +// Send a test update notification email. Uses the dedicated +// `version_update_test` template (migration 087) rather than reusing +// `version_update_available`, so admins on the latest version can still +// verify their SMTP + recipient config — the previous handler bailed +// with "No updates available" when nothing was pending (#418). router.post('/updates/notifications/send', adminAuth, requirePermission('settings.edit'), async (req, res) => { try { - const result = await sendUpdateNotificationNow(); + const result = await sendTestUpdateNotification(); res.json(result); } catch (error) { - logger.error('Error sending update notification:', error); - res.status(500).json({ error: 'Failed to send update notification' }); + logger.error('Error sending test update notification:', error); + res.status(500).json({ error: 'Failed to send test update notification' }); } }); diff --git a/backend/src/services/updateNotificationService.js b/backend/src/services/updateNotificationService.js index d1563925..81422107 100644 --- a/backend/src/services/updateNotificationService.js +++ b/backend/src/services/updateNotificationService.js @@ -172,75 +172,82 @@ async function checkAndNotifyUpdates() { } /** - * Force send update notification (for manual trigger from admin UI) + * Send a TEST notification email to the configured recipients (manual + * trigger from the admin "Send Test Email" button on the Update + * Notifications page). + * + * Uses the version_update_test template (migration 087) which is + * explicitly labelled as a configuration check rather than a real update + * notice. Crucially this path does NOT require updateAvailable to be + * true — it sends regardless of whether the instance is on the latest + * version, so admins can verify their SMTP + recipient list work before + * an actual update lands (#418). + * + * Does NOT update last_notified_version — that field is owned by the + * real-update path so a test send doesn't shadow a future genuine + * notification for the same version. */ -async function sendUpdateNotificationNow() { - logger.info('Manually triggering update notification...'); +async function sendTestUpdateNotification() { + logger.info('Sending test update notification email...'); try { - // Check for available updates - const updateInfo = await checkForUpdates(true); // Force refresh - - if (!updateInfo.updateAvailable) { - return { success: false, message: 'No updates available' }; - } - - const newVersion = updateInfo.latest.forChannel; const settings = await getUpdateNotificationSettings(); - // Get recipients const recipients = await getNotificationRecipients(settings.recipients); - if (recipients.length === 0) { return { success: false, message: 'No recipients configured' }; } - // Ensure email transporter is initialized - await initializeTransporter(); + // checkForUpdates is best-effort here — we want the version + channel + // for the email body, but a transient failure shouldn't block the test + // send. Fall back to env-derived defaults so the email still goes out. + let updateInfo; + try { + updateInfo = await checkForUpdates(true); + } catch (error) { + logger.warn('checkForUpdates failed during test send, using fallbacks:', error.message); + updateInfo = { + current: process.env.npm_package_version || 'unknown', + channel: process.env.UPDATE_CHANNEL || 'stable' + }; + } - // Send email to each recipient - const releaseNotesUrl = `https://github.com/the-luap/picpeak/releases/tag/v${newVersion}`; const channelLabel = updateInfo.channel === 'beta' ? 'Beta' : 'Stable'; + await initializeTransporter(); + let successCount = 0; let errorCount = 0; for (const email of recipients) { try { - await sendTemplateEmail(email, 'version_update_available', { + await sendTemplateEmail(email, 'version_update_test', { current_version: updateInfo.current, - new_version: newVersion, channel: channelLabel, - release_notes_url: releaseNotesUrl + recipient_email: email }); successCount++; } catch (error) { errorCount++; - logger.error(`Failed to send update notification to ${email}:`, error); + logger.error(`Failed to send test update notification to ${email}:`, error); } } - // Update last notified version - if (successCount > 0) { - await updateLastNotifiedVersion(newVersion); - } - return { success: successCount > 0, - newVersion, successCount, errorCount, totalRecipients: recipients.length }; } catch (error) { - logger.error('Error sending manual update notification:', error); + logger.error('Error sending test update notification:', error); return { success: false, message: error.message }; } } module.exports = { checkAndNotifyUpdates, - sendUpdateNotificationNow, + sendTestUpdateNotification, getUpdateNotificationSettings, getNotificationRecipients };