diff --git a/frontend-modern/src/api/notifications.ts b/frontend-modern/src/api/notifications.ts index a412da776..76a8944c9 100644 --- a/frontend-modern/src/api/notifications.ts +++ b/frontend-modern/src/api/notifications.ts @@ -43,6 +43,7 @@ export interface EmailConfig { to: string[]; tls: boolean; startTLS: boolean; + rateLimit?: number; } export interface Webhook { @@ -107,12 +108,13 @@ export class NotificationsAPI { to: (backendConfig.to as string[]) || [], tls: (backendConfig.tls as boolean) || false, startTLS: (backendConfig.startTLS as boolean) || false, + rateLimit: (backendConfig.rateLimit as number) || undefined, }; } static async updateEmailConfig(config: EmailConfig): Promise<{ success: boolean }> { // Backend expects fields with these names (server, port) - const backendConfig = { + const backendConfig: Record = { enabled: config.enabled, server: config.server, port: config.port, @@ -125,6 +127,11 @@ export class NotificationsAPI { provider: config.provider || '', }; + // Only include rateLimit if it's explicitly set + if (config.rateLimit !== undefined) { + backendConfig.rateLimit = config.rateLimit; + } + return apiFetchJSON(`${this.baseUrl}/email`, { method: 'PUT', body: JSON.stringify(backendConfig), diff --git a/frontend-modern/src/components/Settings/DiagnosticsPanel.tsx b/frontend-modern/src/components/Settings/DiagnosticsPanel.tsx index d3f025331..c5abf1a7c 100644 --- a/frontend-modern/src/components/Settings/DiagnosticsPanel.tsx +++ b/frontend-modern/src/components/Settings/DiagnosticsPanel.tsx @@ -268,22 +268,26 @@ export const DiagnosticsPanel: Component = () => { const redactString = (s: string): string => s.replace(ipv4Re, '[REDACTED_IP]'); // Redact node hosts - data.nodes = data.nodes.map((node, i) => ({ - ...node, - host: `node-${i + 1}`, - name: `node-${i + 1}`, - id: `node-${i + 1}`, - error: node.error ? redactString(node.error) : undefined, - })); + if (Array.isArray(data.nodes)) { + data.nodes = data.nodes.map((node, i) => ({ + ...node, + host: `node-${i + 1}`, + name: `node-${i + 1}`, + id: `node-${i + 1}`, + error: node.error ? redactString(node.error) : undefined, + })); + } // Redact PBS hosts - data.pbs = data.pbs.map((p, i) => ({ - ...p, - host: `pbs-${i + 1}`, - name: `pbs-${i + 1}`, - id: `pbs-${i + 1}`, - error: p.error ? redactString(p.error) : undefined, - })); + if (Array.isArray(data.pbs)) { + data.pbs = data.pbs.map((p, i) => ({ + ...p, + host: `pbs-${i + 1}`, + name: `pbs-${i + 1}`, + id: `pbs-${i + 1}`, + error: p.error ? redactString(p.error) : undefined, + })); + } // Redact discovery subnets if (data.discovery) { @@ -343,7 +347,9 @@ export const DiagnosticsPanel: Component = () => { } // Redact IPs in error messages - data.errors = data.errors.map(redactString); + if (Array.isArray(data.errors)) { + data.errors = data.errors.map(redactString); + } // Redact IPs from any raw snapshot data that may be present const raw2 = data as any; diff --git a/frontend-modern/src/pages/Alerts.tsx b/frontend-modern/src/pages/Alerts.tsx index d59e69b10..c7c16fa00 100644 --- a/frontend-modern/src/pages/Alerts.tsx +++ b/frontend-modern/src/pages/Alerts.tsx @@ -1370,7 +1370,7 @@ export function Alerts() { replyTo: '', maxRetries: 3, retryDelay: 5, - rateLimit: 60, + rateLimit: emailConfigData.rateLimit ?? 60, }); } catch (emailErr) { logger.error('Failed to load email configuration:', emailErr); @@ -1435,7 +1435,7 @@ export function Alerts() { replyTo: '', maxRetries: 3, retryDelay: 5, - rateLimit: 60, + rateLimit: emailConfigData.rateLimit ?? 60, }); }) .catch((err) => { diff --git a/internal/api/diagnostics.go b/internal/api/diagnostics.go index 4faf65a88..3fc77dae3 100644 --- a/internal/api/diagnostics.go +++ b/internal/api/diagnostics.go @@ -425,6 +425,8 @@ func writeDiagnosticsResponse(w http.ResponseWriter, diag DiagnosticsInfo, cache func (r *Router) computeDiagnostics(ctx context.Context) DiagnosticsInfo { diag := DiagnosticsInfo{ Errors: []string{}, + Nodes: []NodeDiagnostic{}, + PBS: []PBSDiagnostic{}, } // Version info diff --git a/internal/api/notifications.go b/internal/api/notifications.go index c39daf7a3..3cc997ac5 100644 --- a/internal/api/notifications.go +++ b/internal/api/notifications.go @@ -156,6 +156,7 @@ func (h *NotificationHandlers) UpdateEmailConfig(w http.ResponseWriter, r *http. Str("from", config.From). Int("toCount", len(config.To)). Bool("hasPassword", config.Password != ""). + Int("rateLimit", config.RateLimit). Msg("Parsed email config") h.getMonitor(r.Context()).GetNotificationManager().SetEmailConfig(config) diff --git a/internal/api/notifications_test.go b/internal/api/notifications_test.go index 9199321a4..9613c9510 100644 --- a/internal/api/notifications_test.go +++ b/internal/api/notifications_test.go @@ -318,6 +318,7 @@ func TestNotificationHandlers(t *testing.T) { SMTPHost: "smtp.example.com", Password: "newpassword", } + mockManager.On("GetEmailConfig").Return(notifications.EmailConfig{}).Once() mockManager.On("SetEmailConfig", mock.Anything).Return().Once() mockPersistence.On("SaveEmailConfig", mock.Anything).Return(nil).Once() diff --git a/internal/api/rate_limit_integration_test.go b/internal/api/rate_limit_integration_test.go new file mode 100644 index 000000000..3d382ed5c --- /dev/null +++ b/internal/api/rate_limit_integration_test.go @@ -0,0 +1,140 @@ +package api + +import ( + "bytes" + "encoding/json" + "net/http/httptest" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/notifications" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +// TestRateLimitPersistence_FullRoundTrip tests the complete data flow: +// 1. Existing config has RateLimit=120 +// 2. User updates other fields without rateLimit in JSON +// 3. Backend preserves RateLimit=120 +// 4. GET returns RateLimit=120 +func TestRateLimitPersistence_FullRoundTrip(t *testing.T) { + // Setup mocks + mockMonitor := new(MockNotificationMonitor) + mockManager := new(MockNotificationManager) + mockPersistence := new(MockNotificationConfigPersistence) + + mockMonitor.On("GetNotificationManager").Return(mockManager) + mockMonitor.On("GetConfigPersistence").Return(mockPersistence) + + h := NewNotificationHandlers(nil, mockMonitor) + + // Existing config in "database" has RateLimit=120 + existingConfig := notifications.EmailConfig{ + Enabled: true, + SMTPHost: "smtp.example.com", + Password: "secret", + RateLimit: 120, + } + + // Test 1: GET returns the rateLimit + t.Run("GET_returns_rateLimit", func(t *testing.T) { + mockManager.On("GetEmailConfig").Return(existingConfig).Once() + + req := httptest.NewRequest("GET", "/api/notifications/email", nil) + w := httptest.NewRecorder() + h.GetEmailConfig(w, req) + + assert.Equal(t, 200, w.Code) + + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + + // Verify rateLimit is returned (password should be empty for security) + assert.Equal(t, float64(120), resp["rateLimit"]) + assert.Equal(t, "", resp["password"]) // Redacted + }) + + // Test 2: PUT without rateLimit preserves existing value + t.Run("PUT_without_rateLimit_preserves_existing", func(t *testing.T) { + // Return existing config when handler calls GetEmailConfig + mockManager.On("GetEmailConfig").Return(existingConfig).Once() + + // Expect SetEmailConfig to be called WITH RateLimit=120 preserved + mockManager.On("SetEmailConfig", mock.MatchedBy(func(c notifications.EmailConfig) bool { + t.Logf("SetEmailConfig called with RateLimit=%d", c.RateLimit) + return c.RateLimit == 120 && c.SMTPHost == "smtp.newhost.com" + })).Return().Once() + + mockPersistence.On("SaveEmailConfig", mock.MatchedBy(func(c notifications.EmailConfig) bool { + return c.RateLimit == 120 + })).Return(nil).Once() + + // Request body does NOT include rateLimit + payload := map[string]interface{}{ + "enabled": true, + "server": "smtp.newhost.com", + "password": "newpassword", + } + + body, _ := json.Marshal(payload) + req := httptest.NewRequest("PUT", "/api/notifications/email", bytes.NewReader(body)) + w := httptest.NewRecorder() + h.UpdateEmailConfig(w, req) + + assert.Equal(t, 200, w.Code) + mockManager.AssertExpectations(t) + mockPersistence.AssertExpectations(t) + }) + + // Test 3: PUT with rateLimit=0 explicitly sets it to 0 + t.Run("PUT_with_explicit_rateLimit_0_sets_to_0", func(t *testing.T) { + mockManager.On("GetEmailConfig").Return(existingConfig).Once() + + mockManager.On("SetEmailConfig", mock.MatchedBy(func(c notifications.EmailConfig) bool { + t.Logf("SetEmailConfig called with RateLimit=%d", c.RateLimit) + return c.RateLimit == 0 // User explicitly set to 0 + })).Return().Once() + + mockPersistence.On("SaveEmailConfig", mock.Anything).Return(nil).Once() + + // Request body INCLUDES rateLimit: 0 + payload := map[string]interface{}{ + "enabled": true, + "server": "smtp.example.com", + "rateLimit": 0, + } + + body, _ := json.Marshal(payload) + req := httptest.NewRequest("PUT", "/api/notifications/email", bytes.NewReader(body)) + w := httptest.NewRecorder() + h.UpdateEmailConfig(w, req) + + assert.Equal(t, 200, w.Code) + mockManager.AssertExpectations(t) + }) + + // Test 4: PUT with new rateLimit updates it + t.Run("PUT_with_new_rateLimit_updates", func(t *testing.T) { + mockManager.On("GetEmailConfig").Return(existingConfig).Once() + + mockManager.On("SetEmailConfig", mock.MatchedBy(func(c notifications.EmailConfig) bool { + t.Logf("SetEmailConfig called with RateLimit=%d", c.RateLimit) + return c.RateLimit == 60 // User changed to 60 + })).Return().Once() + + mockPersistence.On("SaveEmailConfig", mock.Anything).Return(nil).Once() + + payload := map[string]interface{}{ + "enabled": true, + "server": "smtp.example.com", + "rateLimit": 60, + } + + body, _ := json.Marshal(payload) + req := httptest.NewRequest("PUT", "/api/notifications/email", bytes.NewReader(body)) + w := httptest.NewRecorder() + h.UpdateEmailConfig(w, req) + + assert.Equal(t, 200, w.Code) + mockManager.AssertExpectations(t) + }) +}