fix: use dedicated Twilio fields and add service switch case tests
- Added `accountSid` and `twilioPhoneNumber` fields to Notification type, MongoDB model, and frontend type instead of reusing `address` and `homeserverUrl` - Updated provider, validation, form hook, and form UI to use new fields - Added Pushover and Twilio to notificationsService routing tests (both handleNotifications and sendTestNotification) - All 59 tests passing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9210259511
commit
d5d6797f91
10 files changed
+40
-27
No files matched your search
@@ -64,10 +64,10 @@ function buildDefaults(data: Notification | null): NotificationFormData {
|
||||
return {
|
||||
type: "twilio",
|
||||
notificationName: data.notificationName || "",
|
||||
address: data.address || "",
|
||||
accountSid: data.accountSid || "",
|
||||
accessToken: data.accessToken || "",
|
||||
phone: data.phone || "",
|
||||
homeserverUrl: data.homeserverUrl || "",
|
||||
twilioPhoneNumber: data.twilioPhoneNumber || "",
|
||||
};
|
||||
}
|
||||
if (data?.type === "pushover") {
|
||||
|
||||
@@ -270,9 +270,9 @@ const NotificationsCreatePage = () => {
|
||||
rightContent={
|
||||
<Stack spacing={theme.spacing(8)}>
|
||||
<Controller
|
||||
name="address"
|
||||
name="accountSid"
|
||||
control={control}
|
||||
defaultValue={"address" in defaults ? defaults.address : ""}
|
||||
defaultValue={"accountSid" in defaults ? defaults.accountSid : ""}
|
||||
render={({ field, fieldState }) => (
|
||||
<TextField
|
||||
{...field}
|
||||
@@ -306,9 +306,11 @@ const NotificationsCreatePage = () => {
|
||||
)}
|
||||
/>
|
||||
<Controller
|
||||
name="homeserverUrl"
|
||||
name="twilioPhoneNumber"
|
||||
control={control}
|
||||
defaultValue={"homeserverUrl" in defaults ? defaults.homeserverUrl : ""}
|
||||
defaultValue={
|
||||
"twilioPhoneNumber" in defaults ? defaults.twilioPhoneNumber : ""
|
||||
}
|
||||
render={({ field, fieldState }) => (
|
||||
<TextField
|
||||
{...field}
|
||||
|
||||
@@ -23,6 +23,8 @@ export interface Notification {
|
||||
homeserverUrl?: string;
|
||||
roomId?: string;
|
||||
accessToken?: string;
|
||||
accountSid?: string;
|
||||
twilioPhoneNumber?: string;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
}
|
||||
@@ -64,10 +64,10 @@ const pushoverSchema = baseSchema.extend({
|
||||
|
||||
const twilioSchema = baseSchema.extend({
|
||||
type: z.literal("twilio"),
|
||||
address: z.string().min(1, "Account SID is required"),
|
||||
accountSid: z.string().min(1, "Account SID is required"),
|
||||
accessToken: z.string().min(1, "Auth token is required"),
|
||||
phone: z.string().min(1, "Recipient phone number is required"),
|
||||
homeserverUrl: z.string().min(1, "Twilio phone number is required"),
|
||||
twilioPhoneNumber: z.string().min(1, "Twilio phone number is required"),
|
||||
});
|
||||
|
||||
export const notificationSchema = z.discriminatedUnion("type", [
|
||||
|
||||
@@ -37,6 +37,8 @@ const NotificationSchema = new Schema<NotificationDocument>(
|
||||
homeserverUrl: { type: String },
|
||||
roomId: { type: String },
|
||||
accessToken: { type: String },
|
||||
accountSid: { type: String },
|
||||
twilioPhoneNumber: { type: String },
|
||||
},
|
||||
{
|
||||
timestamps: true,
|
||||
|
||||
@@ -7,18 +7,18 @@ import got from "got";
|
||||
|
||||
export class TwilioProvider extends NotificationProvider {
|
||||
async sendTestAlert(notification: Partial<Notification>): Promise<boolean> {
|
||||
if (!notification.address || !notification.accessToken || !notification.phone || !notification.homeserverUrl) {
|
||||
if (!notification.accountSid || !notification.accessToken || !notification.phone || !notification.twilioPhoneNumber) {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
await got.post(`https://api.twilio.com/2010-04-01/Accounts/${notification.address}/Messages.json`, {
|
||||
await got.post(`https://api.twilio.com/2010-04-01/Accounts/${notification.accountSid}/Messages.json`, {
|
||||
form: {
|
||||
To: notification.phone,
|
||||
From: notification.homeserverUrl,
|
||||
From: notification.twilioPhoneNumber,
|
||||
Body: getTestMessage(),
|
||||
},
|
||||
username: notification.address,
|
||||
username: notification.accountSid,
|
||||
password: notification.accessToken,
|
||||
...this.gotRequestOptions(),
|
||||
});
|
||||
@@ -38,20 +38,20 @@ export class TwilioProvider extends NotificationProvider {
|
||||
}
|
||||
|
||||
async sendMessage(notification: Notification, message: NotificationMessage): Promise<boolean> {
|
||||
if (!notification.address || !notification.accessToken || !notification.phone || !notification.homeserverUrl) {
|
||||
if (!notification.accountSid || !notification.accessToken || !notification.phone || !notification.twilioPhoneNumber) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const text = this.buildSmsText(message);
|
||||
|
||||
try {
|
||||
await got.post(`https://api.twilio.com/2010-04-01/Accounts/${notification.address}/Messages.json`, {
|
||||
await got.post(`https://api.twilio.com/2010-04-01/Accounts/${notification.accountSid}/Messages.json`, {
|
||||
form: {
|
||||
To: notification.phone,
|
||||
From: notification.homeserverUrl,
|
||||
From: notification.twilioPhoneNumber,
|
||||
Body: text,
|
||||
},
|
||||
username: notification.address,
|
||||
username: notification.accountSid,
|
||||
password: notification.accessToken,
|
||||
...this.gotRequestOptions(),
|
||||
});
|
||||
|
||||
@@ -23,6 +23,8 @@ export interface Notification {
|
||||
homeserverUrl?: string;
|
||||
roomId?: string;
|
||||
accessToken?: string;
|
||||
accountSid?: string;
|
||||
twilioPhoneNumber?: string;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
}
|
||||
@@ -83,10 +83,10 @@ export const createNotificationBodyValidation = z.discriminatedUnion("type", [
|
||||
z.object({
|
||||
notificationName: z.string().min(1, "Notification name is required"),
|
||||
type: z.literal("twilio"),
|
||||
address: z.string().min(1, "Account SID is required"),
|
||||
accountSid: z.string().min(1, "Account SID is required"),
|
||||
accessToken: z.string().min(1, "Auth token is required"),
|
||||
phone: z.string().min(1, "Recipient phone number is required"),
|
||||
homeserverUrl: z.string().min(1, "Twilio phone number is required"),
|
||||
twilioPhoneNumber: z.string().min(1, "Twilio phone number is required"),
|
||||
}),
|
||||
]);
|
||||
|
||||
|
||||
@@ -16,7 +16,8 @@ const createProvider = () => {
|
||||
const makeTwilioNotification = (overrides?: Record<string, unknown>) =>
|
||||
makeNotification({
|
||||
phone: "+15559876543",
|
||||
homeserverUrl: "+15551234567",
|
||||
twilioPhoneNumber: "+15551234567",
|
||||
accountSid: "ACtest123",
|
||||
...overrides,
|
||||
});
|
||||
|
||||
@@ -44,8 +45,8 @@ describe("TwilioProvider", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("returns false when address is missing", async () => {
|
||||
expect(await createProvider().provider.sendTestAlert(makeTwilioNotification({ address: "" }))).toBe(false);
|
||||
it("returns false when accountSid is missing", async () => {
|
||||
expect(await createProvider().provider.sendTestAlert(makeTwilioNotification({ accountSid: "" }))).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when accessToken is missing", async () => {
|
||||
@@ -56,8 +57,8 @@ describe("TwilioProvider", () => {
|
||||
expect(await createProvider().provider.sendTestAlert(makeTwilioNotification({ phone: "" }))).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when homeserverUrl (from number) is missing", async () => {
|
||||
expect(await createProvider().provider.sendTestAlert(makeTwilioNotification({ homeserverUrl: "" }))).toBe(false);
|
||||
it("returns false when twilioPhoneNumber (from number) is missing", async () => {
|
||||
expect(await createProvider().provider.sendTestAlert(makeTwilioNotification({ twilioPhoneNumber: "" }))).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false and logs on error", async () => {
|
||||
@@ -85,8 +86,8 @@ describe("TwilioProvider", () => {
|
||||
expect(form.From).toBe("+15551234567");
|
||||
});
|
||||
|
||||
it("returns false when address is missing", async () => {
|
||||
expect(await createProvider().provider.sendMessage(makeTwilioNotification({ address: "" }) as any, makeMessage())).toBe(false);
|
||||
it("returns false when accountSid is missing", async () => {
|
||||
expect(await createProvider().provider.sendMessage(makeTwilioNotification({ accountSid: "" }) as any, makeMessage())).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when accessToken is missing", async () => {
|
||||
|
||||
@@ -147,7 +147,7 @@ describe("NotificationsService", () => {
|
||||
});
|
||||
|
||||
it("routes to correct provider for each notification type", async () => {
|
||||
const types = ["webhook", "slack", "matrix", "pager_duty", "discord", "email", "teams", "telegram"] as const;
|
||||
const types = ["webhook", "slack", "matrix", "pager_duty", "discord", "email", "teams", "telegram", "pushover", "twilio"] as const;
|
||||
for (const type of types) {
|
||||
const deps = createService();
|
||||
(deps.notificationsRepository.findNotificationsByIds as jest.Mock).mockResolvedValue([makeNotification({ type })]);
|
||||
@@ -163,6 +163,8 @@ describe("NotificationsService", () => {
|
||||
email: deps.emailProvider,
|
||||
teams: deps.teamsProvider,
|
||||
telegram: deps.telegramProvider,
|
||||
pushover: deps.pushoverProvider,
|
||||
twilio: deps.twilioProvider,
|
||||
};
|
||||
expect(providerMap[type].sendMessage).toHaveBeenCalledTimes(1);
|
||||
}
|
||||
@@ -236,7 +238,7 @@ describe("NotificationsService", () => {
|
||||
// ── sendTestNotification ─────────────────────────────────────────────────
|
||||
|
||||
describe("sendTestNotification", () => {
|
||||
it.each([["email"], ["slack"], ["discord"], ["pager_duty"], ["matrix"], ["webhook"], ["teams"], ["telegram"]] as const)(
|
||||
it.each([["email"], ["slack"], ["discord"], ["pager_duty"], ["matrix"], ["webhook"], ["teams"], ["telegram"], ["pushover"], ["twilio"]] as const)(
|
||||
"routes %s to the correct provider",
|
||||
async (type) => {
|
||||
const deps = createService();
|
||||
@@ -254,6 +256,8 @@ describe("NotificationsService", () => {
|
||||
email: deps.emailProvider,
|
||||
teams: deps.teamsProvider,
|
||||
telegram: deps.telegramProvider,
|
||||
pushover: deps.pushoverProvider,
|
||||
twilio: deps.twilioProvider,
|
||||
};
|
||||
expect(providerMap[type].sendTestAlert).toHaveBeenCalledWith(notification);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user