Merge pull request #420 from the-luap/fix/update-notification-test-email-418

fix(admin): test email always sends, regardless of update availability (#418)

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: `
+<h2>This is a test email</h2>
+
+<p>You are receiving this message because an administrator clicked
+<strong>Send Test Email</strong> on the Update Notifications page of your
+PicPeak installation.</p>
+
+<div style="background-color: #f0f8ff; border-left: 4px solid #5C8762; padding: 20px; margin: 20px 0; border-radius: 4px;">
+  <p style="margin: 0;"><strong>Installed version:</strong> {{current_version}}</p>
+  <p style="margin: 10px 0 0 0;"><strong>Channel:</strong> {{channel}}</p>
+  <p style="margin: 10px 0 0 0;"><strong>Recipient address:</strong> {{recipient_email}}</p>
+</div>
+
+<p>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.</p>
+
+<p style="color: #666; font-size: 13px; margin-top: 30px;">No action is required.
+You may safely delete this message.</p>
+
+<p>Best regards,<br>
+Your PicPeak Installation</p>`,
+    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: `
+<h2>Dies ist eine Test-E-Mail</h2>
+
+<p>Sie erhalten diese Nachricht, weil ein Administrator auf der Seite
+"Update-Benachrichtigungen" Ihrer PicPeak-Installation auf
+<strong>Test-E-Mail senden</strong> geklickt hat.</p>
+
+<div style="background-color: #f0f8ff; border-left: 4px solid #5C8762; padding: 20px; margin: 20px 0; border-radius: 4px;">
+  <p style="margin: 0;"><strong>Installierte Version:</strong> {{current_version}}</p>
+  <p style="margin: 10px 0 0 0;"><strong>Kanal:</strong> {{channel}}</p>
+  <p style="margin: 10px 0 0 0;"><strong>Empfänger-Adresse:</strong> {{recipient_email}}</p>
+</div>
+
+<p>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.</p>
+
+<p style="color: #666; font-size: 13px; margin-top: 30px;">Es ist keine Aktion
+erforderlich. Sie können diese Nachricht gefahrlos löschen.</p>
+
+<p>Mit freundlichen Grüßen,<br>
+Ihre PicPeak-Installation</p>`,
+    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();
+};

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' });
   }
 });
 

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
 };