diff --git a/app/public/electron.js b/app/public/electron.js index 859facc5d..d8c082aa7 100644 --- a/app/public/electron.js +++ b/app/public/electron.js @@ -17,7 +17,12 @@ import { willLaunchAtStartup, refreshWillLaunchAtStartup, } from "./auto-launch.js"; -import { setNotificationLevel, getNotificationLevel } from "./notifications.js"; +import { + setNotificationLevel, + getNotificationLevel, + LevelDisabled, + LevelWarningsAndErrors, +} from "./notifications.js"; import { serverForRepo } from "./server.js"; import { loadConfigs, @@ -549,21 +554,26 @@ app.on("ready", () => { function showRepoNotification(e) { const nl = getNotificationLevel(); - if (nl === 0) { + if (nl === LevelDisabled) { // notifications disabled return; } - if (e.severity < 10 && nl === 1) { - // non-important notifications disabled. + const severity = e.notification.severity; + if (severity < 10 && nl === LevelWarningsAndErrors) { + log.info( + "showRepoNotification", + "skipping notification", + e.notification.subject, + ); return; } let urgency = "normal"; - if (e.severity < 0) { + if (severity < 0) { urgency = "low"; - } else if (e.severity >= 10) { + } else if (severity >= 10) { // warnings and errors urgency = "critical"; } else { diff --git a/app/public/notifications.js b/app/public/notifications.js index e7fff1e01..4857982f2 100644 --- a/app/public/notifications.js +++ b/app/public/notifications.js @@ -4,9 +4,9 @@ import { configDir } from "./config.js"; const path = await import("path"); const fs = await import("fs"); -const LevelDisabled = 0; -const LevelDefault = 1; -const LevelAll = 2; +export const LevelDisabled = 0; +export const LevelWarningsAndErrors = 1; +export const LevelAll = 2; let level = -1; @@ -16,7 +16,7 @@ export function getNotificationLevel() { const cfg = fs.readFileSync(path.join(configDir(), "notifications.json")); return JSON.parse(cfg).level; } catch (e) { - level = LevelDefault; + level = LevelWarningsAndErrors; } } @@ -26,7 +26,7 @@ export function getNotificationLevel() { export function setNotificationLevel(l) { level = l; if (level < LevelDisabled || level > LevelAll) { - level = LevelDefault; + level = LevelWarningsAndErrors; } fs.writeFileSync( diff --git a/cli/command_snapshot_create.go b/cli/command_snapshot_create.go index bd2d61af3..d0c90f72c 100644 --- a/cli/command_snapshot_create.go +++ b/cli/command_snapshot_create.go @@ -150,7 +150,7 @@ func (c *commandSnapshotCreate) run(ctx context.Context, rep repo.RepositoryWrit } if c.sendSnapshotReport { - notification.Send(ctx, rep, "snapshot-report", st, notification.SeverityReport, c.svc.notificationTemplateOptions()) + notification.Send(ctx, rep, "snapshot-report", st, c.reportSeverity(st), c.svc.notificationTemplateOptions()) } // ensure we flush at least once in the session to properly close all pending buffers, @@ -173,6 +173,17 @@ func (c *commandSnapshotCreate) run(ctx context.Context, rep repo.RepositoryWrit return errors.Errorf("encountered %v errors:\n%v", len(finalErrors), strings.Join(finalErrors, "\n")) } +func (c *commandSnapshotCreate) reportSeverity(st notifydata.MultiSnapshotStatus) notification.Severity { + switch st.OverallStatusCode() { + case notifydata.StatusCodeFatal: + return notification.SeverityError + case notifydata.StatusCodeWarnings: + return notification.SeverityWarning + default: + return notification.SeverityReport + } +} + func getTags(tagStrings []string) (map[string]string, error) { numberOfPartsInTagString := 2 // tagKeyPrefix is the prefix for user defined tag keys. diff --git a/internal/server/server.go b/internal/server/server.go index e49867f5e..aed1d21ac 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -572,7 +572,19 @@ func (s *Server) sendSnapshotReport(st notifydata.MultiSnapshotStatus) { // send the notification without blocking if we still have the repository // it's possible that repository was closed in the meantime. if rep != nil { - notification.Send(s.rootctx, rep, "snapshot-report", st, notification.SeverityReport, s.notificationTemplateOptions()) + notification.Send(s.rootctx, rep, "snapshot-report", st, s.reportSeverity(st), s.notificationTemplateOptions()) + } +} + +func (*Server) reportSeverity(st notifydata.MultiSnapshotStatus) notification.Severity { + // TODO - this is a duplicate of the code in command_snapshot_create.go - we should unify this. + switch st.OverallStatusCode() { + case notifydata.StatusCodeFatal: + return notification.SeverityError + case notifydata.StatusCodeWarnings: + return notification.SeverityWarning + default: + return notification.SeverityReport } } diff --git a/notification/notifydata/multi_snapshot_status.go b/notification/notifydata/multi_snapshot_status.go index ef89973b6..44a9ec65c 100644 --- a/notification/notifydata/multi_snapshot_status.go +++ b/notification/notifydata/multi_snapshot_status.go @@ -164,6 +164,30 @@ func (m MultiSnapshotStatus) EventArgsType() grpcapi.NotificationEventArgType { return grpcapi.NotificationEventArgType_ARG_TYPE_MULTI_SNAPSHOT_STATUS } +// OverallStatusCode returns the overall status of the snapshots (StatusCodeSuccess, StatusCodeWarnings, or StatusCodeFatal). +func (m MultiSnapshotStatus) OverallStatusCode() string { + var hasWarnings, hasErrors bool + + for _, s := range m.Snapshots { + switch s.StatusCode() { + case StatusCodeFatal: + hasErrors = true + case StatusCodeWarnings: + hasWarnings = true + } + } + + if hasErrors { + return StatusCodeFatal + } + + if hasWarnings { + return StatusCodeWarnings + } + + return StatusCodeSuccess +} + // OverallStatus returns the overall status of the snapshots. func (m MultiSnapshotStatus) OverallStatus() string { var ( diff --git a/notification/notifydata/multi_snapshot_status_test.go b/notification/notifydata/multi_snapshot_status_test.go index 734cc9e01..30fe8d92f 100644 --- a/notification/notifydata/multi_snapshot_status_test.go +++ b/notification/notifydata/multi_snapshot_status_test.go @@ -14,9 +14,10 @@ func TestOverallStatus(t *testing.T) { tests := []struct { - name string - snapshots []*notifydata.ManifestWithError - expected string + name string + snapshots []*notifydata.ManifestWithError + expected string + expectedCode string }{ { name: "one success", @@ -29,7 +30,8 @@ func TestOverallStatus(t *testing.T) { }, }}, }, - expected: "Successfully created a snapshot of /some/path", + expected: "Successfully created a snapshot of /some/path", + expectedCode: notifydata.StatusCodeSuccess, }, { name: "all success", @@ -37,7 +39,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{}}, {Manifest: snapshot.Manifest{}}, }, - expected: "Successfully created 2 snapshots", + expected: "Successfully created 2 snapshots", + expectedCode: notifydata.StatusCodeSuccess, }, { name: "one fatal error", @@ -45,7 +48,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{}, Error: "fatal error"}, {Manifest: snapshot.Manifest{}}, }, - expected: "Failed to create 1 of 2 snapshots", + expected: "Failed to create 1 of 2 snapshots", + expectedCode: notifydata.StatusCodeFatal, }, { name: "one fatal error", @@ -58,7 +62,8 @@ func TestOverallStatus(t *testing.T) { }, }, Error: "fatal error"}, }, - expected: "Failed to create a snapshot of /some/path", + expected: "Failed to create a snapshot of /some/path", + expectedCode: notifydata.StatusCodeFatal, }, { name: "multiple fatal errors", @@ -66,7 +71,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{}, Error: "fatal error"}, {Manifest: snapshot.Manifest{}, Error: "fatal error"}, }, - expected: "Failed to create 2 of 2 snapshots", + expected: "Failed to create 2 of 2 snapshots", + expectedCode: notifydata.StatusCodeFatal, }, { name: "one error", @@ -74,7 +80,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{RootEntry: &snapshot.DirEntry{DirSummary: &fs.DirectorySummary{IgnoredErrorCount: 1}}}}, {Manifest: snapshot.Manifest{}}, }, - expected: "Successfully created 2 snapshots", + expected: "Successfully created 2 snapshots", + expectedCode: notifydata.StatusCodeWarnings, }, { name: "one fatal error and two errors", @@ -84,7 +91,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{RootEntry: &snapshot.DirEntry{DirSummary: &fs.DirectorySummary{IgnoredErrorCount: 1}}}}, {Manifest: snapshot.Manifest{RootEntry: &snapshot.DirEntry{DirSummary: &fs.DirectorySummary{IgnoredErrorCount: 1}}}}, }, - expected: "Failed to create 1 of 4 snapshots", + expected: "Failed to create 1 of 4 snapshots", + expectedCode: notifydata.StatusCodeFatal, }, { name: "one fatal error and one errors", @@ -93,7 +101,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{}}, {Manifest: snapshot.Manifest{RootEntry: &snapshot.DirEntry{DirSummary: &fs.DirectorySummary{IgnoredErrorCount: 1}}}}, }, - expected: "Failed to create 1 of 3 snapshots", + expected: "Failed to create 1 of 3 snapshots", + expectedCode: notifydata.StatusCodeFatal, }, { name: "multiple errors", @@ -101,7 +110,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{RootEntry: &snapshot.DirEntry{DirSummary: &fs.DirectorySummary{IgnoredErrorCount: 1}}}}, {Manifest: snapshot.Manifest{RootEntry: &snapshot.DirEntry{DirSummary: &fs.DirectorySummary{IgnoredErrorCount: 1}}}}, }, - expected: "Successfully created 2 snapshots", + expected: "Successfully created 2 snapshots", + expectedCode: notifydata.StatusCodeWarnings, }, { name: "incomplete snapshot", @@ -109,7 +119,8 @@ func TestOverallStatus(t *testing.T) { {Manifest: snapshot.Manifest{IncompleteReason: "incomplete"}}, {Manifest: snapshot.Manifest{}}, }, - expected: "Successfully created 2 snapshots", + expected: "Successfully created 2 snapshots", + expectedCode: notifydata.StatusCodeSuccess, }, } @@ -117,6 +128,7 @@ func TestOverallStatus(t *testing.T) { t.Run(tt.name, func(t *testing.T) { mss := notifydata.MultiSnapshotStatus{Snapshots: tt.snapshots} require.Equal(t, tt.expected, mss.OverallStatus()) + require.Equal(t, tt.expectedCode, mss.OverallStatusCode()) testRoundTrip(t, &mss) }) }