hub.MakeLink method to assure URLs are formatted properly (#805)

- Updated AlertManager to replace direct app references with a hub interface.
- Changed AlertManager.app to AlertManager.hub
- Add tests for MakeLink

beszel/internal/alerts/alerts.go

@@ -15,8 +15,13 @@ import (
 	"github.com/pocketbase/pocketbase/tools/mailer"
 )
 
+type hubLike interface {
+	core.App
+	MakeLink(parts ...string) string
+}
+
 type AlertManager struct {
-	app           core.App
+	hub           hubLike
 	alertQueue    chan alertTask
 	stopChan      chan struct{}
 	pendingAlerts sync.Map
@@ -79,9 +84,9 @@ var supportsTitle = map[string]struct{}{
 }
 
 // NewAlertManager creates a new AlertManager instance.
-func NewAlertManager(app core.App) *AlertManager {
+func NewAlertManager(app hubLike) *AlertManager {
 	am := &AlertManager{
-		app:        app,
+		hub:        app,
 		alertQueue: make(chan alertTask),
 		stopChan:   make(chan struct{}),
 	}
@@ -91,7 +96,7 @@ func NewAlertManager(app core.App) *AlertManager {
 
 func (am *AlertManager) SendAlert(data AlertMessageData) error {
 	// get user settings
-	record, err := am.app.FindFirstRecordByFilter(
+	record, err := am.hub.FindFirstRecordByFilter(
 		"user_settings", "user={:user}",
 		dbx.Params{"user": data.UserID},
 	)
@@ -104,12 +109,12 @@ func (am *AlertManager) SendAlert(data AlertMessageData) error {
 		Webhooks: []string{},
 	}
 	if err := record.UnmarshalJSONField("settings", &userAlertSettings); err != nil {
-		am.app.Logger().Error("Failed to unmarshal user settings", "err", err.Error())
+		am.hub.Logger().Error("Failed to unmarshal user settings", "err", err)
 	}
 	// send alerts via webhooks
 	for _, webhook := range userAlertSettings.Webhooks {
 		if err := am.SendShoutrrrAlert(webhook, data.Title, data.Message, data.Link, data.LinkText); err != nil {
-			am.app.Logger().Error("Failed to send shoutrrr alert", "err", err.Error())
+			am.hub.Logger().Error("Failed to send shoutrrr alert", "err", err)
 		}
 	}
 	// send alerts via email
@@ -125,15 +130,15 @@ func (am *AlertManager) SendAlert(data AlertMessageData) error {
 		Subject: data.Title,
 		Text:    data.Message + fmt.Sprintf("\n\n%s", data.Link),
 		From: mail.Address{
-			Address: am.app.Settings().Meta.SenderAddress,
-			Name:    am.app.Settings().Meta.SenderName,
+			Address: am.hub.Settings().Meta.SenderAddress,
+			Name:    am.hub.Settings().Meta.SenderName,
 		},
 	}
-	err = am.app.NewMailClient().Send(&message)
+	err = am.hub.NewMailClient().Send(&message)
 	if err != nil {
 		return err
 	}
-	am.app.Logger().Info("Sent email alert", "to", message.To, "subj", message.Subject)
+	am.hub.Logger().Info("Sent email alert", "to", message.To, "subj", message.Subject)
 	return nil
 }
 
@@ -183,9 +188,9 @@ func (am *AlertManager) SendShoutrrrAlert(notificationUrl, title, message, link,
 	err = shoutrrr.Send(parsedURL.String(), message)
 
 	if err == nil {
-		am.app.Logger().Info("Sent shoutrrr alert", "title", title)
+		am.hub.Logger().Info("Sent shoutrrr alert", "title", title)
 	} else {
-		am.app.Logger().Error("Error sending shoutrrr alert", "err", err.Error())
+		am.hub.Logger().Error("Error sending shoutrrr alert", "err", err)
 		return err
 	}
 	return nil
@@ -201,7 +206,7 @@ func (am *AlertManager) SendTestNotification(e *core.RequestEvent) error {
 	if url == "" {
 		return e.JSON(200, map[string]string{"err": "URL is required"})
 	}
-	err := am.SendShoutrrrAlert(url, "Test Alert", "This is a notification from Beszel.", am.app.Settings().Meta.AppURL, "View Beszel")
+	err := am.SendShoutrrrAlert(url, "Test Alert", "This is a notification from Beszel.", am.hub.Settings().Meta.AppURL, "View Beszel")
 	if err != nil {
 		return e.JSON(200, map[string]string{"err": err.Error()})
 	}

beszel/internal/alerts/alerts_status.go

@@ -2,7 +2,6 @@ package alerts
 
 import (
 	"fmt"
-	"net/url"
 	"strings"
 	"time"
 
@@ -87,7 +86,7 @@ func (am *AlertManager) HandleStatusAlerts(newStatus string, systemRecord *core.
 
 // getSystemStatusAlerts retrieves all "Status" alert records for a given system ID.
 func (am *AlertManager) getSystemStatusAlerts(systemID string) ([]*core.Record, error) {
-	alertRecords, err := am.app.FindAllRecords("alerts", dbx.HashExp{
+	alertRecords, err := am.hub.FindAllRecords("alerts", dbx.HashExp{
 		"system": systemID,
 		"name":   "Status",
 	})
@@ -130,7 +129,7 @@ func (am *AlertManager) handleSystemUp(systemName string, alertRecords []*core.R
 		}
 		// No alert scheduled for this record, send "up" alert
 		if err := am.sendStatusAlert("up", systemName, alertRecord); err != nil {
-			am.app.Logger().Error("Failed to send alert", "err", err.Error())
+			am.hub.Logger().Error("Failed to send alert", "err", err)
 		}
 	}
 }
@@ -147,7 +146,7 @@ func (am *AlertManager) sendStatusAlert(alertStatus string, systemName string, a
 	title := fmt.Sprintf("Connection to %s is %s %v", systemName, alertStatus, emoji)
 	message := strings.TrimSuffix(title, emoji)
 
-	if errs := am.app.ExpandRecord(alertRecord, []string{"user"}, nil); len(errs) > 0 {
+	if errs := am.hub.ExpandRecord(alertRecord, []string{"user"}, nil); len(errs) > 0 {
 		return errs["user"]
 	}
 	user := alertRecord.ExpandedOne("user")
@@ -159,7 +158,7 @@ func (am *AlertManager) sendStatusAlert(alertStatus string, systemName string, a
 		UserID:   user.Id,
 		Title:    title,
 		Message:  message,
-		Link:     am.app.Settings().Meta.AppURL + "/system/" + url.PathEscape(systemName),
+		Link:     am.hub.MakeLink("systems", systemName),
 		LinkText: "View " + systemName,
 	})
 }

beszel/internal/alerts/alerts_system.go

@@ -3,7 +3,6 @@ package alerts
 import (
 	"beszel/internal/entities/system"
 	"fmt"
-	"net/url"
 	"strings"
 	"time"
 
@@ -15,7 +14,7 @@ import (
 )
 
 func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *system.CombinedData) error {
-	alertRecords, err := am.app.FindAllRecords("alerts",
+	alertRecords, err := am.hub.FindAllRecords("alerts",
 		dbx.NewExp("system={:system}", dbx.Params{"system": systemRecord.Id}),
 	)
 	if err != nil || len(alertRecords) == 0 {
@@ -101,7 +100,7 @@ func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *syst
 		Created types.DateTime `db:"created"`
 	}{}
 
-	err = am.app.DB().
+	err = am.hub.DB().
 		Select("stats", "created").
 		From("system_stats").
 		Where(dbx.NewExp(
@@ -271,12 +270,12 @@ func (am *AlertManager) sendSystemAlert(alert SystemAlertData) {
 	body := fmt.Sprintf("%s averaged %.2f%s for the previous %v %s.", alert.descriptor, alert.val, alert.unit, alert.min, minutesLabel)
 
 	alert.alertRecord.Set("triggered", alert.triggered)
-	if err := am.app.Save(alert.alertRecord); err != nil {
-		// app.Logger().Error("failed to save alert record", "err", err.Error())
+	if err := am.hub.Save(alert.alertRecord); err != nil {
+		// app.Logger().Error("failed to save alert record", "err", err)
 		return
 	}
 	// expand the user relation and send the alert
-	if errs := am.app.ExpandRecord(alert.alertRecord, []string{"user"}, nil); len(errs) > 0 {
+	if errs := am.hub.ExpandRecord(alert.alertRecord, []string{"user"}, nil); len(errs) > 0 {
 		// app.Logger().Error("failed to expand user relation", "errs", errs)
 		return
 	}
@@ -285,7 +284,7 @@ func (am *AlertManager) sendSystemAlert(alert SystemAlertData) {
 			UserID:   user.Id,
 			Title:    subject,
 			Message:  body,
-			Link:     am.app.Settings().Meta.AppURL + "/system/" + url.PathEscape(systemName),
+			Link:     am.hub.MakeLink("systems", systemName),
 			LinkText: "View " + systemName,
 		})
 	}

beszel/internal/hub/hub.go

@@ -285,3 +285,16 @@ func (h *Hub) GetSSHKey() (ssh.Signer, error) {
 
 	return sshPrivate, err
 }
+
+// MakeLink formats a link with the app URL and path segments.
+// Only path segments should be provided.
+func (h *Hub) MakeLink(parts ...string) string {
+	base := strings.TrimSuffix(h.Settings().Meta.AppURL, "/")
+	for _, part := range parts {
+		if part == "" {
+			continue
+		}
+		base = fmt.Sprintf("%s/%s", base, url.PathEscape(part))
+	}
+	return base
+}

beszel/internal/hub/hub_test.go

@@ -0,0 +1,106 @@
+//go:build testing
+// +build testing
+
+package hub
+
+import (
+	"testing"
+
+	"github.com/pocketbase/pocketbase"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMakeLink(t *testing.T) {
+	// The Hub's MakeLink method uses h.Settings().Meta.AppURL.
+	// h.Settings() is a method on h.App (of type core.App).
+	// We use a pocketbase.PocketBase instance as core.App and set its Meta.AppURL
+	// directly for isolated testing of MakeLink.
+	app := pocketbase.New()
+	h := NewHub(app)
+
+	tests := []struct {
+		name     string
+		appURL   string
+		parts    []string
+		expected string
+	}{
+		{
+			name:     "no parts, no trailing slash in AppURL",
+			appURL:   "http://localhost:8090",
+			parts:    []string{},
+			expected: "http://localhost:8090",
+		},
+		{
+			name:     "no parts, with trailing slash in AppURL",
+			appURL:   "http://localhost:8090/",
+			parts:    []string{},
+			expected: "http://localhost:8090", // TrimSuffix should handle the trailing slash
+		},
+		{
+			name:     "one part",
+			appURL:   "http://example.com",
+			parts:    []string{"one"},
+			expected: "http://example.com/one",
+		},
+		{
+			name:     "multiple parts",
+			appURL:   "http://example.com",
+			parts:    []string{"alpha", "beta", "gamma"},
+			expected: "http://example.com/alpha/beta/gamma",
+		},
+		{
+			name:     "parts with spaces needing escaping",
+			appURL:   "http://example.com",
+			parts:    []string{"path with spaces", "another part"},
+			expected: "http://example.com/path%20with%20spaces/another%20part",
+		},
+		{
+			name:     "parts with slashes needing escaping",
+			appURL:   "http://example.com",
+			parts:    []string{"a/b", "c"},
+			expected: "http://example.com/a%2Fb/c", // url.PathEscape escapes '/'
+		},
+		{
+			name:     "AppURL with subpath, no trailing slash",
+			appURL:   "http://localhost/sub",
+			parts:    []string{"resource"},
+			expected: "http://localhost/sub/resource",
+		},
+		{
+			name:     "AppURL with subpath, with trailing slash",
+			appURL:   "http://localhost/sub/",
+			parts:    []string{"item"},
+			expected: "http://localhost/sub/item",
+		},
+		{
+			name:     "empty parts in the middle",
+			appURL:   "http://localhost",
+			parts:    []string{"first", "", "third"},
+			expected: "http://localhost/first/third",
+		},
+		{
+			name:     "leading and trailing empty parts",
+			appURL:   "http://localhost",
+			parts:    []string{"", "path", ""},
+			expected: "http://localhost/path",
+		},
+		{
+			name:     "parts with various special characters",
+			appURL:   "https://test.dev/",
+			parts:    []string{"p@th?", "key=value&"},
+			expected: "https://test.dev/p@th%3F/key=value&",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Store original and defer restoration if app instance is reused across test functions (good practice)
+			originalAppURL := app.Settings().Meta.AppURL
+			app.Settings().Meta.AppURL = tt.appURL
+			defer func() { app.Settings().Meta.AppURL = originalAppURL }()
+
+			got := h.MakeLink(tt.parts...)
+			assert.Equal(t, tt.expected, got, "MakeLink generated URL does not match expected")
+		})
+	}
+}