Skip to content
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

feat(notification): Handle long running transaction and add status support #8900

Merged
merged 12 commits into from
Sep 1, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
ContainerRegistrationKeys,
ModuleRegistrationName,
} from "@medusajs/utils"
import { TestEventUtils, medusaIntegrationTestRunner } from "medusa-test-utils"
import { medusaIntegrationTestRunner, TestEventUtils } from "medusa-test-utils"

jest.setTimeout(50000)

Expand Down Expand Up @@ -91,14 +91,18 @@ medusaIntegrationTestRunner({
it("should throw an exception if there is no provider for the channel", async () => {
const notification = {
to: "test@medusajs.com",
template: "order-created",
channel: "sms",
} as CreateNotificationDTO

const error = await service
.createNotifications(notification)
.catch((e) => e)

const [notificationResult] = await service.listNotifications()

expect(error.message).toEqual(
"Could not find a notification provider for channel: sms"
`Could not find a notification provider for channel: sms for notification: ${notificationResult.id}`
)
})

Expand Down
4 changes: 4 additions & 0 deletions packages/core/types/src/notification/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ export interface NotificationDTO {
* The date and time the notification was created.
*/
created_at: Date
/**
* The status of the notification
*/
status: "pending" | "success" | "failure"
}

/**
Expand Down
5 changes: 5 additions & 0 deletions packages/core/utils/src/notification/common.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export enum NotificationStatus {
PENDING = "pending",
SUCCESS = "success",
FAILURE = "failure",
}
1 change: 1 addition & 0 deletions packages/core/utils/src/notification/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from "./abstract-notification-provider"
export * from "./events"
export * from "./common"
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ export class NotificationProviderServiceFixtures extends AbstractNotificationPro
async send(
notification: NotificationTypes.ProviderSendNotificationDTO
): Promise<NotificationTypes.ProviderSendNotificationResultsDTO> {
if (notification.to === "fail") {
throw new Error("Failed to send notification")
}
return { id: "external_id" }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import {
Module,
Modules,
NotificationEvents,
NotificationStatus,
} from "@medusajs/utils"
import {
MockEventBusService,
moduleIntegrationTestRunner,
SuiteOptions,
} from "medusa-test-utils"
import { resolve } from "path"
import { NotificationModuleService } from "@services"
Expand All @@ -32,10 +32,10 @@ let moduleOptions = {

jest.setTimeout(30000)

moduleIntegrationTestRunner({
moduleIntegrationTestRunner<INotificationModuleService>({
moduleName: Modules.NOTIFICATION,
moduleOptions,
testSuite: ({ service }: SuiteOptions<INotificationModuleService>) =>
testSuite: ({ service }) =>
describe("Notification Module Service", () => {
let eventBusEmitSpy

Expand Down Expand Up @@ -66,7 +66,7 @@ moduleIntegrationTestRunner({
})
})

it("sends a notification and stores it in the database", async () => {
it("should send a notification and stores it in the database", async () => {
const notification = {
to: "admin@medusa.com",
template: "some-template",
Expand All @@ -79,11 +79,12 @@ moduleIntegrationTestRunner({
expect.objectContaining({
provider_id: "test-provider",
external_id: "external_id",
status: NotificationStatus.SUCCESS,
})
)
})

it("emits an event when a notification is created", async () => {
it("should emit an event when a notification is created", async () => {
const notification = {
to: "admin@medusa.com",
template: "some-template",
Expand All @@ -109,7 +110,7 @@ moduleIntegrationTestRunner({
)
})

it("ensures the same notification is not sent twice", async () => {
it("should ensures the same notification is not sent twice", async () => {
const notification = {
to: "admin@medusa.com",
template: "some-template",
Expand All @@ -123,11 +124,86 @@ moduleIntegrationTestRunner({
expect.objectContaining({
provider_id: "test-provider",
external_id: "external_id",
status: NotificationStatus.SUCCESS,
})
)

const secondResult = await service.createNotifications(notification)
expect(secondResult).toBe(undefined)
})

it("should manage the status of multiple notification properly in any scenarios", async () => {
const notification1 = {
to: "admin@medusa.com",
template: "some-template",
channel: "email",
data: {},
idempotency_key: "idempotency-key",
}

const notification2 = {
to: "0000000000",
template: "some-template",
channel: "sms",
data: {},
idempotency_key: "idempotency-key-2",
}

const notification3 = {
to: "admin@medusa.com",
template: "some-template",
channel: "email",
data: {},
idempotency_key: "idempotency-key-3",
}

const notification4 = {
to: "fail",
template: "some-template",
channel: "email",
data: {},
idempotency_key: "idempotency-key-4",
}

const err = await service
.createNotifications([
notification1,
notification2,
notification3,
notification4,
])
.catch((e) => e)

const notifications = await service.listNotifications()

expect(notifications).toHaveLength(4)

const notification1Result = notifications.find(
(n) => n.idempotency_key === "idempotency-key"
)!
expect(notification1Result.status).toEqual(NotificationStatus.SUCCESS)

const notification2Result = notifications.find(
(n) => n.idempotency_key === "idempotency-key-2"
)!
expect(notification2Result.status).toEqual(NotificationStatus.FAILURE)

const notification3Result = notifications.find(
(n) => n.idempotency_key === "idempotency-key-3"
)!
expect(notification3Result.status).toEqual(NotificationStatus.SUCCESS)

const notification4Result = notifications.find(
(n) => n.idempotency_key === "idempotency-key-4"
)!
expect(notification4Result.status).toEqual(NotificationStatus.FAILURE)

expect(err).toBeTruthy()
expect(err.message).toEqual(
`Could not find a notification provider for channel: sms for notification id ${notification2Result.id}
Failed to send notification with id ${notification4Result.id}:
Failed to send notification`
)
})
}),
})
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,21 @@
"nullable": true,
"mappedType": "text"
},
"status": {
"name": "status",
"type": "text",
"unsigned": false,
"autoincrement": false,
"primary": false,
"nullable": false,
"default": "'pending'",
"enumItems": [
"pending",
"success",
"failure"
],
"mappedType": "enum"
},
"provider_id": {
"name": "provider_id",
"type": "text",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Migration } from '@mikro-orm/migrations';

export class Migration20240830094712 extends Migration {

async up(): Promise<void> {
this.addSql('alter table if exists "notification" add column if not exists "status" text check ("status" in (\'pending\', \'success\', \'failure\')) not null default \'pending\';');
}

async down(): Promise<void> {
this.addSql('alter table if exists "notification" drop column if exists "status";');
}

}
5 changes: 4 additions & 1 deletion packages/modules/notification/src/models/notification.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { model } from "@medusajs/utils"
import { model, NotificationStatus } from "@medusajs/utils"
import { NotificationProvider } from "./notification-provider"

// We probably want to have a TTL for each entry, so we don't bloat the DB (and also for GDPR reasons if TTL < 30 days).
Expand All @@ -24,6 +24,9 @@ export const Notification = model.define("notification", {
idempotency_key: model.text().unique().nullable(),
// The ID of the notification in the external system, if applicable
external_id: model.text().nullable(),
// The status of the notification
status: model.enum(NotificationStatus).default(NotificationStatus.PENDING),

provider: model
.belongsTo(() => NotificationProvider, { mappedBy: "notifications" })
.nullable(),
Expand Down
Loading
Loading