feat: enhance workspace notification summary handling with improved error logging and retry logic

2026-04-06 00:32:12 +02:00 · 2026-03-24 14:31:29 +00:00
parent 20d21a40b7
commit b434f1fef8
4 changed files with 285 additions and 53 deletions
--- a/App/FeatureSet/Dashboard/src/Components/Workspace/WorkspaceSummaryTable.tsx
+++ b/App/FeatureSet/Dashboard/src/Components/Workspace/WorkspaceSummaryTable.tsx
@@ -207,7 +207,7 @@ const WorkspaceSummaryTable: FunctionComponent<ComponentProps> = (
      setTestError(undefined);

      const response: HTTPResponse<EmptyResponseData> | HTTPErrorResponse =
-        await API.get({
+        await API.post({
          url: URL.fromString(APP_API_URL.toString()).addRoute(
            `/workspace-notification-summary/test/${summaryId.toString()}`,
          ),
@@ -286,9 +286,19 @@ const WorkspaceSummaryTable: FunctionComponent<ComponentProps> = (
          values.projectId = ProjectUtil.getCurrentProjectId()!;
          values.workspaceType = props.workspaceType;

-          // Set nextSendAt: use sendFirstReportAt if provided, otherwise compute from interval
+          // Set nextSendAt based on sendFirstReportAt or recurringInterval
          if (values.sendFirstReportAt) {
-            values.nextSendAt = values.sendFirstReportAt;
+            const firstReportDate: Date = new Date(
+              values.sendFirstReportAt as unknown as string,
+            );
+            if (
+              firstReportDate.getTime() >
+              OneUptimeDate.getCurrentDate().getTime()
+            ) {
+              values.nextSendAt = firstReportDate;
+            } else {
+              values.nextSendAt = values.sendFirstReportAt;
+            }
          } else if (values.recurringInterval) {
            const recurring: Recurring = Recurring.fromJSON(
              values.recurringInterval,
@@ -328,7 +338,17 @@ const WorkspaceSummaryTable: FunctionComponent<ComponentProps> = (
          if (values.filters && Array.isArray(values.filters)) {
            values.filters = values.filters.filter(
              (f: NotificationRuleCondition) => {
-                return f.value && Array.isArray(f.value) && f.value.length > 0;
+                if (!f.value) {
+                  return false;
+                }
+                if (Array.isArray(f.value)) {
+                  return f.value.length > 0;
+                }
+                // String-based conditions (e.g., title contains "X")
+                if (typeof f.value === "string") {
+                  return f.value.trim().length > 0;
+                }
+                return true;
              },
            );
          }
@@ -340,41 +360,11 @@ const WorkspaceSummaryTable: FunctionComponent<ComponentProps> = (
          return Promise.resolve(values);
        }}
        onBeforeEdit={(values: WorkspaceNotificationSummary) => {
-          // Parse channel names from comma-separated string
-          if (values.channelNames && typeof values.channelNames === "string") {
-            values.channelNames = (values.channelNames as unknown as string)
-              .split(",")
-              .map((name: string) => {
-                return name.trim();
-              })
-              .filter((name: string) => {
-                return name.length > 0;
-              });
-          }
-
-          /*
-           * If sendFirstReportAt was changed and is in the future, use it as nextSendAt.
-           * Otherwise leave nextSendAt alone — the worker manages it after the first send.
-           */
-          if (values.sendFirstReportAt) {
-            const firstReportDate: Date = new Date(
-              values.sendFirstReportAt as unknown as string,
-            );
-            if (
-              firstReportDate.getTime() >
-              OneUptimeDate.getCurrentDate().getTime()
-            ) {
-              values.nextSendAt = firstReportDate;
-            }
-          }
-
-          // Clean up empty filters
-          if (values.filters && Array.isArray(values.filters)) {
-            values.filters = values.filters.filter(
-              (f: NotificationRuleCondition) => {
-                return f.value && Array.isArray(f.value) && f.value.length > 0;
-              },
-            );
+          // Convert channelNames from JSON array to comma-separated string for the text input
+          if (values.channelNames && Array.isArray(values.channelNames)) {
+            values.channelNames = (values.channelNames as Array<string>).join(
+              ", ",
+            ) as unknown as Array<string>;
          }

          return Promise.resolve(values);
@@ -414,6 +404,22 @@ const WorkspaceSummaryTable: FunctionComponent<ComponentProps> = (
            required: true,
            placeholder: "#incidents-summary, #engineering",
          },
+          ...(props.workspaceType === WorkspaceType.MicrosoftTeams
+            ? [
+                {
+                  field: {
+                    teamName: true,
+                  },
+                  stepId: "basic",
+                  title: "Team Name",
+                  description:
+                    "The name of the Microsoft Teams team where the summary will be posted.",
+                  fieldType: FormFieldSchemaType.Text,
+                  required: false,
+                  placeholder: "Engineering Team",
+                },
+              ]
+            : []),
          {
            field: {
              isEnabled: true,
@@ -686,9 +692,19 @@ const WorkspaceSummaryTable: FunctionComponent<ComponentProps> = (
        <ConfirmModal
          title={testError ? `Test Failed` : `Summary Sent`}
          error={testError}
-          description={`The test summary was sent successfully. Check your ${getWorkspaceTypeDisplayName(props.workspaceType)} channel to see how it looks.`}
+          description={
+            testError
+              ? `The test summary could not be sent. Please check your channel names and workspace connection settings.`
+              : `The test summary was sent successfully. Check your ${getWorkspaceTypeDisplayName(props.workspaceType)} channel to see how it looks.`
+          }
          submitButtonType={ButtonStyleType.NORMAL}
          submitButtonText={"Close"}
+          onClose={() => {
+            setShowTestSuccessModal(false);
+            setTestSummary(undefined);
+            setShowTestModal(false);
+            setTestError("");
+          }}
          onSubmit={async () => {
            setShowTestSuccessModal(false);
            setTestSummary(undefined);
--- a/Common/Server/API/WorkspaceNotificationSummaryAPI.ts
+++ b/Common/Server/API/WorkspaceNotificationSummaryAPI.ts
@@ -13,6 +13,7 @@ import CommonAPI from "./CommonAPI";
 import DatabaseCommonInteractionProps from "../../Types/BaseDatabase/DatabaseCommonInteractionProps";
 import WorkspaceNotificationSummary from "../../Models/DatabaseModels/WorkspaceNotificationSummary";
 import ObjectID from "../../Types/ObjectID";
+import BadDataException from "../../Types/Exception/BadDataException";

 export default class WorkspaceNotificationSummaryAPI extends BaseAPI<
  WorkspaceNotificationSummary,
@@ -21,7 +22,7 @@ export default class WorkspaceNotificationSummaryAPI extends BaseAPI<
  public constructor() {
    super(WorkspaceNotificationSummary, WorkspaceNotificationSummaryService);

-    this.router.get(
+    this.router.post(
      `${new this.entityType().getCrudApiPath()?.toString()}/test/:workspaceNotificationSummaryId`,
      UserMiddleware.getUserMiddleware,
      async (req: ExpressRequest, res: ExpressResponse, next: NextFunction) => {
@@ -29,10 +30,28 @@ export default class WorkspaceNotificationSummaryAPI extends BaseAPI<
          const databaseProps: DatabaseCommonInteractionProps =
            await CommonAPI.getDatabaseCommonInteractionProps(req);

+          const summaryId: ObjectID = new ObjectID(
+            req.params["workspaceNotificationSummaryId"] as string,
+          );
+
+          // Verify the summary belongs to the user's project
+          const summary: WorkspaceNotificationSummary | null =
+            await this.service.findOneById({
+              id: summaryId,
+              select: { projectId: true },
+              props: databaseProps,
+            });
+
+          if (!summary) {
+            return Response.sendErrorResponse(
+              req,
+              res,
+              new BadDataException("Summary not found or access denied"),
+            );
+          }
+
          await this.service.testSummary({
-            summaryId: new ObjectID(
-              req.params["workspaceNotificationSummaryId"] as string,
-            ),
+            summaryId: summaryId,
            props: databaseProps,
            projectId: databaseProps.tenantId!,
            testByUserId: databaseProps.userId!,
--- a/Common/Server/Services/WorkspaceNotificationSummaryService.ts
+++ b/Common/Server/Services/WorkspaceNotificationSummaryService.ts
@@ -17,6 +17,11 @@ import AlertEpisode from "../../Models/DatabaseModels/AlertEpisode";
 import IncidentStateTimeline from "../../Models/DatabaseModels/IncidentStateTimeline";
 import AlertStateTimeline from "../../Models/DatabaseModels/AlertStateTimeline";
 import Label from "../../Models/DatabaseModels/Label";
+import Monitor from "../../Models/DatabaseModels/Monitor";
+import WorkspaceNotificationLogService from "./WorkspaceNotificationLogService";
+import WorkspaceNotificationStatus from "../../Types/Workspace/WorkspaceNotificationStatus";
+import WorkspaceNotificationActionType from "../../Types/Workspace/WorkspaceNotificationActionType";
+import logger from "../Utils/Logger";
 import OneUptimeDate from "../../Types/Date";
 import QueryHelper from "../Types/Database/QueryHelper";
 import WorkspaceMessagePayload, {
@@ -124,10 +129,53 @@ export class Service extends DatabaseService<WorkspaceNotificationSummary> {
      teamId: summary.teamName || undefined,
    };

-    await WorkspaceUtil.postMessageToAllWorkspaceChannelsAsBot({
-      projectId: summary.projectId,
-      messagePayloadsByWorkspace: [messagePayload],
-    });
+    try {
+      await WorkspaceUtil.postMessageToAllWorkspaceChannelsAsBot({
+        projectId: summary.projectId,
+        messagePayloadsByWorkspace: [messagePayload],
+      });
+
+      // Log successful send
+      for (const channelName of summary.channelNames) {
+        await WorkspaceNotificationLogService.createWorkspaceLog(
+          {
+            projectId: summary.projectId!,
+            workspaceType: summary.workspaceType!,
+            channelName: channelName,
+            actionType: WorkspaceNotificationActionType.SendMessage,
+            status: WorkspaceNotificationStatus.Success,
+            statusMessage: data.isTest
+              ? "Test summary sent successfully"
+              : "Summary sent successfully",
+            message: `${summary.summaryType || ""} summary "${summary.name || "Untitled"}" sent to channel "${channelName}"`,
+          },
+          { isRoot: true },
+        );
+      }
+    } catch (err) {
+      // Log failed send
+      for (const channelName of summary.channelNames) {
+        await WorkspaceNotificationLogService.createWorkspaceLog(
+          {
+            projectId: summary.projectId!,
+            workspaceType: summary.workspaceType!,
+            channelName: channelName,
+            actionType: WorkspaceNotificationActionType.SendMessage,
+            status: WorkspaceNotificationStatus.Error,
+            statusMessage: err instanceof Error ? err.message : "Unknown error",
+            message: `Failed to send ${summary.summaryType || ""} summary "${summary.name || "Untitled"}" to channel "${channelName}"`,
+          },
+          { isRoot: true },
+        ).catch((logErr: unknown) => {
+          logger.error(
+            "Failed to create workspace notification log for summary send failure",
+          );
+          logger.error(logErr);
+        });
+      }
+
+      throw err; // Re-throw so caller knows it failed
+    }

    if (!data.isTest) {
      await this.updateOneById({
@@ -242,7 +290,7 @@ export class Service extends DatabaseService<WorkspaceNotificationSummary> {
          return l._id?.toString() || "";
        }) || [],
      [NotificationRuleConditionCheckOn.Monitors]:
-        incident.monitors?.map((m: Incident) => {
+        incident.monitors?.map((m: Monitor) => {
          return m._id?.toString() || "";
        }) || [],
      // unused for incidents
@@ -329,6 +377,109 @@ export class Service extends DatabaseService<WorkspaceNotificationSummary> {
    };
  }

+  // Build values map for an incident episode
+  private static buildIncidentEpisodeValues(episode: IncidentEpisode): {
+    [key in NotificationRuleConditionCheckOn]:
+      | string
+      | Array<string>
+      | undefined;
+  } {
+    return {
+      [NotificationRuleConditionCheckOn.IncidentEpisodeTitle]:
+        episode.title || "",
+      [NotificationRuleConditionCheckOn.IncidentEpisodeDescription]:
+        episode.description || "",
+      [NotificationRuleConditionCheckOn.IncidentEpisodeSeverity]:
+        episode.incidentSeverity?._id?.toString() || "",
+      [NotificationRuleConditionCheckOn.IncidentEpisodeState]:
+        episode.currentIncidentState?._id?.toString() || "",
+      [NotificationRuleConditionCheckOn.IncidentEpisodeLabels]:
+        episode.labels?.map((l: Label) => {
+          return l._id?.toString() || "";
+        }) || [],
+      // unused for incident episodes
+      [NotificationRuleConditionCheckOn.IncidentTitle]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentDescription]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentSeverity]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentState]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentLabels]: undefined,
+      [NotificationRuleConditionCheckOn.AlertTitle]: undefined,
+      [NotificationRuleConditionCheckOn.AlertDescription]: undefined,
+      [NotificationRuleConditionCheckOn.AlertSeverity]: undefined,
+      [NotificationRuleConditionCheckOn.AlertState]: undefined,
+      [NotificationRuleConditionCheckOn.AlertLabels]: undefined,
+      [NotificationRuleConditionCheckOn.AlertEpisodeTitle]: undefined,
+      [NotificationRuleConditionCheckOn.AlertEpisodeDescription]: undefined,
+      [NotificationRuleConditionCheckOn.AlertEpisodeSeverity]: undefined,
+      [NotificationRuleConditionCheckOn.AlertEpisodeState]: undefined,
+      [NotificationRuleConditionCheckOn.AlertEpisodeLabels]: undefined,
+      [NotificationRuleConditionCheckOn.Monitors]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorName]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorType]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorStatus]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorLabels]: undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceTitle]: undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceDescription]:
+        undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceState]: undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceLabels]: undefined,
+      [NotificationRuleConditionCheckOn.OnCallDutyPolicyName]: undefined,
+      [NotificationRuleConditionCheckOn.OnCallDutyPolicyDescription]: undefined,
+      [NotificationRuleConditionCheckOn.OnCallDutyPolicyLabels]: undefined,
+    };
+  }
+
+  // Build values map for an alert episode
+  private static buildAlertEpisodeValues(episode: AlertEpisode): {
+    [key in NotificationRuleConditionCheckOn]:
+      | string
+      | Array<string>
+      | undefined;
+  } {
+    return {
+      [NotificationRuleConditionCheckOn.AlertEpisodeTitle]: episode.title || "",
+      [NotificationRuleConditionCheckOn.AlertEpisodeDescription]:
+        episode.description || "",
+      [NotificationRuleConditionCheckOn.AlertEpisodeSeverity]:
+        episode.alertSeverity?._id?.toString() || "",
+      [NotificationRuleConditionCheckOn.AlertEpisodeState]:
+        episode.currentAlertState?._id?.toString() || "",
+      [NotificationRuleConditionCheckOn.AlertEpisodeLabels]:
+        episode.labels?.map((l: Label) => {
+          return l._id?.toString() || "";
+        }) || [],
+      // unused for alert episodes
+      [NotificationRuleConditionCheckOn.IncidentTitle]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentDescription]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentSeverity]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentState]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentLabels]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentEpisodeTitle]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentEpisodeDescription]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentEpisodeSeverity]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentEpisodeState]: undefined,
+      [NotificationRuleConditionCheckOn.IncidentEpisodeLabels]: undefined,
+      [NotificationRuleConditionCheckOn.AlertTitle]: undefined,
+      [NotificationRuleConditionCheckOn.AlertDescription]: undefined,
+      [NotificationRuleConditionCheckOn.AlertSeverity]: undefined,
+      [NotificationRuleConditionCheckOn.AlertState]: undefined,
+      [NotificationRuleConditionCheckOn.AlertLabels]: undefined,
+      [NotificationRuleConditionCheckOn.Monitors]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorName]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorType]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorStatus]: undefined,
+      [NotificationRuleConditionCheckOn.MonitorLabels]: undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceTitle]: undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceDescription]:
+        undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceState]: undefined,
+      [NotificationRuleConditionCheckOn.ScheduledMaintenanceLabels]: undefined,
+      [NotificationRuleConditionCheckOn.OnCallDutyPolicyName]: undefined,
+      [NotificationRuleConditionCheckOn.OnCallDutyPolicyDescription]: undefined,
+      [NotificationRuleConditionCheckOn.OnCallDutyPolicyLabels]: undefined,
+    };
+  }
+
  // ───────────────────────── main builder ─────────────────────────

  @CaptureSpan()
@@ -706,7 +857,7 @@ export class Service extends DatabaseService<WorkspaceNotificationSummary> {
  }): Promise<void> {
    const { blocks, items, fromDate, projectId } = data;

-    const episodes: Array<IncidentEpisode> =
+    let episodes: Array<IncidentEpisode> =
      await IncidentEpisodeService.findAllBy({
        query: {
          projectId,
@@ -722,12 +873,24 @@ export class Service extends DatabaseService<WorkspaceNotificationSummary> {
            _id: true,
            isResolvedState: true,
          },
+          labels: { _id: true, name: true },
          createdAt: true,
          resolvedAt: true,
        },
        props: { isRoot: true },
      });

+    // Apply filters
+    if (data.filters && data.filters.length > 0) {
+      episodes = episodes.filter((ep: IncidentEpisode) => {
+        return Service.matchesFilters({
+          filters: data.filters,
+          filterCondition: data.filterCondition,
+          values: Service.buildIncidentEpisodeValues(ep),
+        });
+      });
+    }
+
    const dashboardUrl: URL = await DatabaseConfig.getDashboardUrl();

    if (Service.has(items, WorkspaceNotificationSummaryItem.TotalCount)) {
@@ -1123,7 +1286,7 @@ export class Service extends DatabaseService<WorkspaceNotificationSummary> {
  }): Promise<void> {
    const { blocks, items, fromDate, projectId } = data;

-    const episodes: Array<AlertEpisode> = await AlertEpisodeService.findAllBy({
+    let episodes: Array<AlertEpisode> = await AlertEpisodeService.findAllBy({
      query: {
        projectId,
        createdAt: QueryHelper.greaterThanEqualTo(fromDate),
@@ -1134,12 +1297,24 @@ export class Service extends DatabaseService<WorkspaceNotificationSummary> {
        description: true,
        alertSeverity: { name: true, _id: true },
        currentAlertState: { name: true, _id: true, isResolvedState: true },
+        labels: { _id: true, name: true },
        createdAt: true,
        resolvedAt: true,
      },
      props: { isRoot: true },
    });

+    // Apply filters
+    if (data.filters && data.filters.length > 0) {
+      episodes = episodes.filter((ep: AlertEpisode) => {
+        return Service.matchesFilters({
+          filters: data.filters,
+          filterCondition: data.filterCondition,
+          values: Service.buildAlertEpisodeValues(ep),
+        });
+      });
+    }
+
    const dashboardUrl: URL = await DatabaseConfig.getDashboardUrl();

    if (Service.has(items, WorkspaceNotificationSummaryItem.TotalCount)) {
--- a/Worker/Jobs/WorkspaceNotificationSummary/SendSummary.ts
+++ b/Worker/Jobs/WorkspaceNotificationSummary/SendSummary.ts
@@ -32,8 +32,8 @@ RunCron(

    for (const summary of summariesToSend) {
      try {
-        // Calculate next send time based on recurring interval
-        const nextSendAt: Date = Recurring.getNextDate(
+        // Calculate next send time using calendar-correct math
+        const nextSendAt: Date = Recurring.getNextDateInterval(
          summary.nextSendAt!,
          summary.recurringInterval!,
        );
@@ -61,6 +61,28 @@ RunCron(
          `WorkspaceNotificationSummary:SendSummary: Error sending summary ${summary.id?.toString()}`,
        );
        logger.error(err);
+
+        // Roll back nextSendAt so it will be retried on the next cron run
+        try {
+          await WorkspaceNotificationSummaryService.updateOneById({
+            id: summary.id!,
+            data: {
+              nextSendAt: summary.nextSendAt!,
+            },
+            props: {
+              isRoot: true,
+            },
+          });
+
+          logger.debug(
+            `WorkspaceNotificationSummary:SendSummary: Rolled back nextSendAt for summary ${summary.id?.toString()} — will retry on next cron run`,
+          );
+        } catch (rollbackErr) {
+          logger.error(
+            `WorkspaceNotificationSummary:SendSummary: Failed to roll back nextSendAt for summary ${summary.id?.toString()}`,
+          );
+          logger.error(rollbackErr);
+        }
      }
    }
  },