From b434f1fef8b65bd33eef1086e29e242944aed3e5 Mon Sep 17 00:00:00 2001 From: Nawaz Dhandala Date: Tue, 24 Mar 2026 14:31:29 +0000 Subject: [PATCH] feat: enhance workspace notification summary handling with improved error logging and retry logic --- .../Workspace/WorkspaceSummaryTable.tsx | 96 +++++---- .../API/WorkspaceNotificationSummaryAPI.ts | 27 ++- .../WorkspaceNotificationSummaryService.ts | 189 +++++++++++++++++- .../SendSummary.ts | 26 ++- 4 files changed, 285 insertions(+), 53 deletions(-) diff --git a/App/FeatureSet/Dashboard/src/Components/Workspace/WorkspaceSummaryTable.tsx b/App/FeatureSet/Dashboard/src/Components/Workspace/WorkspaceSummaryTable.tsx index b215a38d5b..2ef7513146 100644 --- 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 = ( setTestError(undefined); const response: HTTPResponse | 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 = ( 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 = ( 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 = ( 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).join( + ", ", + ) as unknown as Array; } return Promise.resolve(values); @@ -414,6 +404,22 @@ const WorkspaceSummaryTable: FunctionComponent = ( 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 = ( { + setShowTestSuccessModal(false); + setTestSummary(undefined); + setShowTestModal(false); + setTestError(""); + }} onSubmit={async () => { setShowTestSuccessModal(false); setTestSummary(undefined); diff --git a/Common/Server/API/WorkspaceNotificationSummaryAPI.ts b/Common/Server/API/WorkspaceNotificationSummaryAPI.ts index 3733b5cbb8..7d25cdbb4f 100644 --- 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!, diff --git a/Common/Server/Services/WorkspaceNotificationSummaryService.ts b/Common/Server/Services/WorkspaceNotificationSummaryService.ts index e48657b70b..0fd9aca6e4 100644 --- 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 { 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 { 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 { }; } + // Build values map for an incident episode + private static buildIncidentEpisodeValues(episode: IncidentEpisode): { + [key in NotificationRuleConditionCheckOn]: + | string + | Array + | 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 + | 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 { }): Promise { const { blocks, items, fromDate, projectId } = data; - const episodes: Array = + let episodes: Array = await IncidentEpisodeService.findAllBy({ query: { projectId, @@ -722,12 +873,24 @@ export class Service extends DatabaseService { _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 { }): Promise { const { blocks, items, fromDate, projectId } = data; - const episodes: Array = await AlertEpisodeService.findAllBy({ + let episodes: Array = await AlertEpisodeService.findAllBy({ query: { projectId, createdAt: QueryHelper.greaterThanEqualTo(fromDate), @@ -1134,12 +1297,24 @@ export class Service extends DatabaseService { 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)) { diff --git a/Worker/Jobs/WorkspaceNotificationSummary/SendSummary.ts b/Worker/Jobs/WorkspaceNotificationSummary/SendSummary.ts index 85c6e99336..0e455cd702 100644 --- 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); + } } } },