-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(api-service): use new workflow run count table in usage page #10074
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
486ecd8
4779b96
924a4cd
a85397d
a07a328
168f20f
8a33c7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,23 @@ | ||
| import { Injectable } from '@nestjs/common'; | ||
| import { InstrumentUsecase, PinoLogger, WorkflowRunRepository } from '@novu/application-generic'; | ||
| import { | ||
| FeatureFlagsService, | ||
| InstrumentUsecase, | ||
| PinoLogger, | ||
| WorkflowRunCountRepository, | ||
| WorkflowRunRepository, | ||
| } from '@novu/application-generic'; | ||
| import { NotificationTemplateRepository } from '@novu/dal'; | ||
| import { FeatureFlagsKeysEnum } from '@novu/shared'; | ||
| import { WorkflowVolumeDataPointDto } from '../../dtos/get-charts.response.dto'; | ||
| import { BuildWorkflowByVolumeChartCommand } from './build-workflow-by-volume-chart.command'; | ||
|
|
||
| @Injectable() | ||
| export class BuildWorkflowByVolumeChart { | ||
| constructor( | ||
| private workflowRunRepository: WorkflowRunRepository, | ||
| private workflowRunCountRepository: WorkflowRunCountRepository, | ||
| private featureFlagsService: FeatureFlagsService, | ||
| private notificationTemplateRepository: NotificationTemplateRepository, | ||
| private logger: PinoLogger | ||
| ) { | ||
| this.logger.setContext(BuildWorkflowByVolumeChart.name); | ||
|
|
@@ -16,6 +27,66 @@ export class BuildWorkflowByVolumeChart { | |
| async execute(command: BuildWorkflowByVolumeChartCommand): Promise<WorkflowVolumeDataPointDto[]> { | ||
| const { environmentId, organizationId, startDate, endDate, workflowIds } = command; | ||
|
|
||
| const isRollupEnabled = await this.featureFlagsService.getFlag({ | ||
| key: FeatureFlagsKeysEnum.IS_WORKFLOW_RUN_TREND_FROM_ROLLUP_ENABLED, | ||
| defaultValue: false, | ||
| organization: { _id: organizationId }, | ||
| environment: { _id: environmentId }, | ||
| }); | ||
|
|
||
| if (isRollupEnabled) { | ||
| return this.buildChartFromWorkflowRunCount(startDate, endDate, environmentId, organizationId); | ||
| } | ||
|
|
||
| return this.buildChartFromWorkflowRuns(startDate, endDate, environmentId, organizationId, workflowIds); | ||
| } | ||
|
|
||
| private async buildChartFromWorkflowRunCount( | ||
| startDate: Date, | ||
| endDate: Date, | ||
| environmentId: string, | ||
| organizationId: string | ||
| ): Promise<WorkflowVolumeDataPointDto[]> { | ||
| const workflowVolumes = await this.workflowRunCountRepository.getWorkflowVolumeData( | ||
| environmentId, | ||
| organizationId, | ||
| startDate, | ||
| endDate | ||
| ); | ||
|
|
||
| if (workflowVolumes.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const triggerIdentifiers = workflowVolumes.map((row) => row.workflow_run_id); | ||
|
|
||
| const templates = await this.notificationTemplateRepository.findByTriggerIdentifierBulk( | ||
| environmentId, | ||
| triggerIdentifiers, | ||
| { select: ['name', 'triggers'] } | ||
| ); | ||
|
|
||
| const nameByIdentifier = new Map<string, string>(); | ||
| for (const template of templates) { | ||
| const identifier = template.triggers?.[0]?.identifier; | ||
| if (identifier) { | ||
| nameByIdentifier.set(identifier, template.name); | ||
| } | ||
|
Comment on lines
+70
to
+74
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Template name mapping assumes the matching trigger is always first Line 71 only reads Suggested fix-const nameByIdentifier = new Map<string, string>();
+const nameByIdentifier = new Map<string, string>();
+const requestedIdentifierSet = new Set(triggerIdentifiers);
for (const template of templates) {
- const identifier = template.triggers?.[0]?.identifier;
- if (identifier) {
- nameByIdentifier.set(identifier, template.name);
+ for (const trigger of template.triggers ?? []) {
+ if (trigger?.identifier && requestedIdentifierSet.has(trigger.identifier)) {
+ nameByIdentifier.set(trigger.identifier, template.name);
+ }
}
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| return workflowVolumes.map((row) => ({ | ||
| workflowName: nameByIdentifier.get(row.workflow_run_id) ?? row.workflow_run_id, | ||
| count: parseInt(row.count, 10), | ||
| })); | ||
| } | ||
|
|
||
| private async buildChartFromWorkflowRuns( | ||
| startDate: Date, | ||
| endDate: Date, | ||
| environmentId: string, | ||
| organizationId: string, | ||
| workflowIds?: string[] | ||
| ): Promise<WorkflowVolumeDataPointDto[]> { | ||
| const workflowRuns = await this.workflowRunRepository.getWorkflowVolumeData( | ||
| environmentId, | ||
| organizationId, | ||
|
|
@@ -24,11 +95,9 @@ export class BuildWorkflowByVolumeChart { | |
| workflowIds | ||
| ); | ||
|
|
||
| const chartData: WorkflowVolumeDataPointDto[] = workflowRuns.map((workflowRun) => ({ | ||
| return workflowRuns.map((workflowRun) => ({ | ||
| workflowName: workflowRun.workflow_name, | ||
| count: parseInt(workflowRun.count, 10), | ||
| })); | ||
|
|
||
| return chartData; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ import { | |
| FeatureFlagsService, | ||
| InstrumentUsecase, | ||
| PinoLogger, | ||
| TraceLogRepository, | ||
| WorkflowRunCountRepository, | ||
| WorkflowRunRepository, | ||
| } from '@novu/application-generic'; | ||
| import { FeatureFlagsKeysEnum } from '@novu/shared'; | ||
|
|
@@ -14,7 +14,7 @@ import { BuildWorkflowRunsTrendChartCommand } from './build-workflow-runs-trend- | |
| export class BuildWorkflowRunsTrendChart { | ||
| constructor( | ||
| private workflowRunRepository: WorkflowRunRepository, | ||
| private traceLogRepository: TraceLogRepository, | ||
| private workflowRunCountRepository: WorkflowRunCountRepository, | ||
| private featureFlagsService: FeatureFlagsService, | ||
| private logger: PinoLogger | ||
| ) { | ||
|
|
@@ -25,33 +25,31 @@ export class BuildWorkflowRunsTrendChart { | |
| async execute(command: BuildWorkflowRunsTrendChartCommand): Promise<WorkflowRunsTrendDataPointDto[]> { | ||
| const { environmentId, organizationId, startDate, endDate, workflowIds } = command; | ||
|
|
||
| const isTraceBasedEnabled = await this.featureFlagsService.getFlag({ | ||
| key: FeatureFlagsKeysEnum.IS_WORKFLOW_RUN_TREND_FROM_TRACES_ENABLED, | ||
| const isRollupEnabled = await this.featureFlagsService.getFlag({ | ||
| key: FeatureFlagsKeysEnum.IS_WORKFLOW_RUN_TREND_FROM_ROLLUP_ENABLED, | ||
| defaultValue: false, | ||
| organization: { _id: organizationId }, | ||
| environment: { _id: environmentId }, | ||
| }); | ||
|
|
||
| if (isTraceBasedEnabled) { | ||
| return this.buildChartFromTraces(startDate, endDate, environmentId, organizationId, workflowIds); | ||
| if (isRollupEnabled) { | ||
| return this.buildChartFromWorkflowRunCount(startDate, endDate, environmentId, organizationId); | ||
| } | ||
|
|
||
| return this.buildChartFromWorkflowRuns(startDate, endDate, environmentId, organizationId, workflowIds); | ||
|
||
| } | ||
|
|
||
| private async buildChartFromTraces( | ||
| private async buildChartFromWorkflowRunCount( | ||
| startDate: Date, | ||
| endDate: Date, | ||
| environmentId: string, | ||
| organizationId: string, | ||
| workflowIds?: string[] | ||
| organizationId: string | ||
| ): Promise<WorkflowRunsTrendDataPointDto[]> { | ||
| const workflowRuns = await this.traceLogRepository.getWorkflowRunsTrendData( | ||
| const workflowRuns = await this.workflowRunCountRepository.getWorkflowRunsTrendData( | ||
| environmentId, | ||
| organizationId, | ||
| startDate, | ||
| endDate, | ||
| workflowIds | ||
| endDate | ||
| ); | ||
|
|
||
| const dataByDate = new Map<string, WorkflowRunsTrendDataPointDto>(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same observation as the trend chart:
workflowIdsis silently ignored when rollup is enabled.When the rollup flag is on,
buildChartFromWorkflowRunCountignores theworkflowIdsparameter. This is the same pattern as inBuildWorkflowRunsTrendChart. If this is expected, consider at minimum adding a log whenworkflowIdsis non-empty but unused.🤖 Prompt for AI Agents