-
-
Notifications
You must be signed in to change notification settings - Fork 271
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
Implemented the handleTestNotification function. #1904
Changes from 1 commit
7b1a60d
845b769
9aa0041
b811fb9
f7b247f
798a680
8e026ed
f735b08
de8edbf
b1194eb
00700c8
8994b4b
182c513
910defb
4cbcebb
13b8bb6
caab2c3
c84a547
3136bdb
840690d
100c5d3
9462632
a0920f9
51da6b1
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,5 +1,7 @@ | ||
import { useState, useMemo } from "react"; | ||
import { useTranslation } from "react-i18next"; | ||
import { toast } from 'react-toastify'; | ||
|
||
import { | ||
Dialog, | ||
DialogContent, | ||
|
@@ -129,11 +131,87 @@ const NotificationIntegrationModal = ({ | |
})); | ||
}; | ||
|
||
const handleTestNotification = (type) => { | ||
console.log(`Testing ${type} notification`); | ||
//implement the test notification functionality | ||
}; | ||
const handleTestNotification = async (type) => { | ||
// Get the notification type details | ||
const notificationType = activeNotificationTypes.find(t => t.id === type); | ||
|
||
if (notificationType === undefined) { | ||
return; | ||
} | ||
|
||
// Helper to get the field state key | ||
const getFieldKey = (typeId, fieldId) => { | ||
return `${typeId}${fieldId.charAt(0).toUpperCase() + fieldId.slice(1)}`; | ||
}; | ||
|
||
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. This function will throw uncaught errors under many conditions. typeId or fieldId undefined, null, not a string etc. Let's make this safer and catch errors. 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. Done. |
||
// Prepare request payload based on notification type | ||
let payload = { platform: type }; | ||
|
||
switch(type) { | ||
case 'slack': | ||
payload.webhookUrl = integrations[getFieldKey('slack', 'webhook')]; | ||
if (!payload.webhookUrl) { | ||
toast.error('Please enter a Slack webhook URL first.'); | ||
return; | ||
} | ||
break; | ||
|
||
case 'discord': | ||
payload.webhookUrl = integrations[getFieldKey('discord', 'webhook')]; | ||
if (!payload.webhookUrl) { | ||
toast.error('Please enter a Discord webhook URL first.'); | ||
return; | ||
} | ||
break; | ||
|
||
case 'telegram': | ||
payload.botToken = integrations[getFieldKey('telegram', 'token')]; | ||
payload.chatId = integrations[getFieldKey('telegram', 'chatId')]; | ||
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. Constant keys should be refactored out
etc. Always best to avoid magic values 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. Done. |
||
if (!payload.botToken || !payload.chatId) { | ||
toast.error('Please enter both Telegram bot token and chat ID.'); | ||
return; | ||
} | ||
break; | ||
|
||
case 'webhook': | ||
payload.webhookUrl = integrations[getFieldKey('webhook', 'url')]; | ||
payload.platform = 'slack'; // Use slack as platform for webhooks | ||
if (!payload.webhookUrl) { | ||
toast.error('Please enter a webhook URL first.'); | ||
return; | ||
} | ||
break; | ||
|
||
default: | ||
toast.error('This notification type cannot be tested.'); | ||
return; | ||
} | ||
|
||
try { | ||
const apiUrl = `${import.meta.env.VITE_APP_API_BASE_URL || 'http://localhost:5000'}/api/v1/notifications/test-webhook`; | ||
|
||
|
||
const response = await fetch(apiUrl, { | ||
method: 'POST', | ||
headers: { | ||
'Content-Type': 'application/json' | ||
}, | ||
body: JSON.stringify(payload), | ||
}); | ||
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. Netwrok request should be made through the They should also be carried out via a hook that provides a loading state and an error state. Please see other components/pages that make network requests for examples 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. Done. |
||
|
||
const data = await response.json(); | ||
|
||
if (response.ok && data.success) { | ||
toast.success('Test notification sent successfully!'); | ||
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. User facing strings need to go through the il8n implementation 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. Done. |
||
} else { | ||
throw new Error(data.msg || 'Failed to send test notification'); | ||
} | ||
} catch (error) { | ||
toast.error(`Failed to send test notification: ${error.message}`); | ||
} | ||
}; | ||
|
||
|
||
const handleSave = () => { | ||
//notifications array for selected integrations | ||
const notifications = [...(monitor?.notifications || [])]; | ||
|
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.
Please use
typeof === "undefined"
, this is a safer way to check for undefined and is the standard for this appThere 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.
Done.