feat(api,worker): timed digest logic #3359

LetItRock · 2023-05-08T23:42:11Z

What change does this PR introduce?

In this PR:

API/Worker changes:

the Timed Digest logic that calculates the job delay by using the class from the private package
updated job schema
updated swagger API types

FE changes:

fixed missing timed digest form field names
use shared enums instead of strings

Tests will be added in a separate PR.

Link to the related enterprise package PR: https://github.com/novuhq/packages-enterprise/pull/2

Why was this change needed?

This PR is a part of the Timed Digest feature.

Other information (Screenshots)

linear · 2023-05-08T23:42:13Z

NV-2221 [API] Update the digesting logic to support the scheduled type

LetItRock · 2023-05-08T23:45:55Z

apps/api/src/app/events/usecases/index.ts

@@ -27,6 +28,7 @@ export const USE_CASES = [
  DigestFilterSteps,
  DigestFilterStepsRegular,
  DigestFilterStepsBackoff,
+  DigestFilterStepsTimed,


the new use-case to filter steps for the timed digest

both worker/api are using it... the api app uses it because of the batch endpoint... the worker app has the logic to handle the trigger events from the queue and parse to the jobs

LetItRock · 2023-05-08T23:46:29Z

apps/api/src/app/shared/dtos/notification-step.ts

+  @ApiPropertyOptional({ enum: [...Object.values(OrdinalEnum)] })
+  ordinal?: OrdinalEnum;

-  @ApiPropertyOptional()
-  ordinalValue?: string;
+  @ApiPropertyOptional({ enum: [...Object.values(OrdinalValueEnum)] })
+  ordinalValue?: OrdinalValueEnum;

-  @ApiPropertyOptional()
-  monthlyType?: string;
+  @ApiPropertyOptional({ enum: [...Object.values(MonthlyTypeEnum)] })
+  monthlyType?: MonthlyTypeEnum;


updated swagger enums and values

LetItRock · 2023-05-08T23:47:00Z

apps/web/src/pages/templates/workflow/digest/ScheduleMonthlyFields.tsx

-            <Radio.Group spacing={0} defaultValue="each" value={radioGroup.value} onChange={radioGroup.onChange}>
+            <Radio.Group
+              spacing={0}
+              defaultValue={MonthlyTypeEnum.EACH}


use enums instead of strings

LetItRock · 2023-05-08T23:47:25Z

apps/web/src/pages/templates/workflow/digest/WillBeSentHeader.tsx

+  const atTime = watch(`steps.${index}.metadata.timed.atTime`);
+  const weekDays = watch(`steps.${index}.metadata.timed.weekDays`) || [];
+  const monthDays = watch(`steps.${index}.metadata.timed.monthDays`) || [];


updated names for these fields that were missed in previous PR

LetItRock · 2023-05-08T23:49:20Z

libs/dal/src/repositories/job/job.schema.ts

+      timed: {
+        atTime: {
+          type: Schema.Types.String,
+        },
+        weekDays: [Schema.Types.String],
+        monthDays: [Schema.Types.Number],
+        ordinal: {
+          type: Schema.Types.String,
+        },
+        ordinalValue: {
+          type: Schema.Types.String,
+        },
+        monthlyType: {
+          type: Schema.Types.String,
+        },
+      },


updated job schema

LetItRock · 2023-05-08T23:53:11Z

packages/application-generic/src/services/calculate-delay/calculate-delay.service.ts

+    if (IS_ENTERPRISE && stepMetadata.type === DigestTypeEnum.TIMED) {
+      const timedDigestMeta = stepMetadata as IDigestTimedMetadata;
+
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      const { TimedDigestDelayService } = require('@novu/digest-schedule');
+
+      return TimedDigestDelayService.calculate({
+        unit: timedDigestMeta.unit,
+        amount: timedDigestMeta.amount,
+        atTime: timedDigestMeta.timed?.atTime,
+        weekDays: timedDigestMeta.timed?.weekDays,
+        monthDays: timedDigestMeta.timed?.monthDays,
+      });
+    }


the calculations for the timed digest delay

LetItRock · 2023-05-08T23:53:42Z

packages/application-generic/src/usecases/add-job/add-digest-job.usecase.ts

@@ -46,32 +44,11 @@ export class AddDigestJob {
  ): Promise<AddDigestJobResult> {
    const { job } = command;

-    this.validateDigest(job);
+    validateDigest(job);


moved to the helper functions outside

LetItRock · 2023-05-08T23:54:06Z

packages/application-generic/src/usecases/add-job/add-digest-job.usecase.ts

-
-    // TODO: timed digest
-    if (
-      this.isRegularDigest(job.digest.type) &&


removed this check that I've added before

LetItRock · 2023-05-08T23:54:55Z

packages/application-generic/src/usecases/add-job/validation.ts

@@ -0,0 +1,127 @@
+import {


the validation helper functions to check the digest metadata values

LetItRock · 2023-05-08T23:57:12Z

...es/application-generic/src/usecases/digest-filter-steps/digest-filter-steps-timed.usecase.ts

+import { DigestFilterSteps } from './digest-filter-steps.usecase';
+
+@Injectable()
+export class DigestFilterStepsTimed {


the use-case to filter steps for the timed digest run... basically at first trigger we will create all the jobs from the template steps; on subsequent triggers if digest is running we will only create the jobs from the steps that are before the digest

How is this different from the regular backoff?

Not much but I've improved it a little.

The regular digest filter use-case: always creates a jobs array based on template steps, even if there is a digest job delayed, and then when the digest is running it marks all other digest jobs as completed. This leads to other jobs in the pending state forever.

The timed digest filter use-case: obviously creates jobs based on the template steps, but it checks if there is a delayed timed digest job, and if there is it just creates a jobs array from the steps that are before the digest, so like it cuts the jobs array.

I've asked this question before on the Discord, but no one has answered it: https://discord.com/channels/895029566685462578/1045289346598703164/1096406448839331941

davidsoderberg · 2023-05-09T07:51:14Z

packages/application-generic/src/services/calculate-delay/calculate-delay.service.ts

    return 0;
  }

-  toMilliseconds(amount: number, unit: DigestUnitEnum): number {
+  private toMilliseconds(amount: number, unit: DigestUnitEnum): number {


why do we make this private?

because it's not used anywhere now only internally

apps/web/src/pages/templates/workflow/digest/ScheduleMonthlyFields.tsx

apps/web/src/pages/templates/workflow/digest/WillBeSentHeader.tsx

enterprise/packages

p-fernandez · 2023-05-09T08:57:05Z

libs/shared/src/entities/step/index.ts

+  SUNDAY = 'sunday',
+  MONDAY = 'monday',
+  TUESDAY = 'tuesday',
+  WEDNESDAY = 'wednesday',
+  THURSDAY = 'thursday',
+  FRIDAY = 'friday',
+  SATURDAY = 'saturday',


How do we plan to use this Enum values? Mostly as day names should be capitalised.

This enum is reused between FE/BE.
On the FE we do use it as values for the dropdown, but the labels are capitalized.
On the BE we do have validation for the request value to be one of the enum values, also it is used for the timed digest calculations.

packages/application-generic/src/usecases/add-job/validation.ts

…te-digesting-logic-to-support-scheduled-type-main

…pport-scheduled-type-main

LetItRock requested review from davidsoderberg and scopsy May 8, 2023 23:42

LetItRock self-assigned this May 8, 2023

github-actions bot added @novu/api @novu/dal @novu/shared @novu/web @novu/worker labels May 8, 2023

feat(api,worker): timed digest logic

089581c

LetItRock force-pushed the nv-2221-update-digesting-logic-to-support-scheduled-type-main branch from 82be2a5 to 089581c Compare May 8, 2023 23:51

LetItRock commented May 8, 2023

View reviewed changes

davidsoderberg reviewed May 9, 2023

View reviewed changes

davidsoderberg approved these changes May 9, 2023

View reviewed changes

p-fernandez reviewed May 9, 2023

View reviewed changes

LetItRock added 2 commits May 9, 2023 14:57

feat(api,worker,web): added suggestions from the pr #3359

4b97fd3

Merge branch 'nv-2226-update-digest-metadata-types' into nv-2221-upda…

032f461

…te-digesting-logic-to-support-scheduled-type-main

Base automatically changed from nv-2226-update-digest-metadata-types to timed-digest May 10, 2023 10:37

Merge branch 'timed-digest' into nv-2221-update-digesting-logic-to-su…

c7e9e9b

…pport-scheduled-type-main

LetItRock merged commit 1f56352 into timed-digest May 10, 2023
29 checks passed

LetItRock deleted the nv-2221-update-digesting-logic-to-support-scheduled-type-main branch May 10, 2023 11:52

linear bot mentioned this pull request Sep 5, 2023

Stacked PRs - add ee-auth package #4107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api,worker): timed digest logic #3359

feat(api,worker): timed digest logic #3359

LetItRock commented May 8, 2023

linear bot commented May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

LetItRock May 8, 2023

davidsoderberg May 9, 2023

LetItRock May 9, 2023

davidsoderberg May 9, 2023

LetItRock May 9, 2023

p-fernandez May 9, 2023

LetItRock May 9, 2023

feat(api,worker): timed digest logic #3359

feat(api,worker): timed digest logic #3359

Conversation

LetItRock commented May 8, 2023

What change does this PR introduce?

Why was this change needed?

Other information (Screenshots)

linear bot commented May 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment