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.

Sign up for GitHub

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

fix: remove service name/type from discord notifier #5181

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

fix: remove service name/type from discord notifier #5181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0d328e7
fix: misspelling of 'address' (#5180)
DeJayDev Oct 9, 2024
c28710c
fix: remove service name/type from discord notifier
DeJayDev Oct 9, 2024
af230ba
Use `msg` rather than parse it out of the heartbeat
DeJayDev Oct 10, 2024
8b82520
Conditionally add service url
DeJayDev Oct 10, 2024
ecac291
Merge branch 'master' into dj/discord-service-url
DeJayDev Oct 10, 2024
cd69146
add discord buttons to payload
DeJayDev Oct 21, 2024
64f5284
Merge branch 'master' into dj/discord-service-url
DeJayDev Oct 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 64 additions & 53 deletions server/notification-providers/discord.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const NotificationProvider = require("./notification-provider");
const axios = require("axios");
const { DOWN, UP } = require("../../src/util");
const { Settings } = require("../settings");
const { DOWN, UP, getMonitorRelativeURL } = require("../../src/util");

class Discord extends NotificationProvider {
name = "discord";
Expand All @@ -19,8 +20,13 @@ class Discord extends NotificationProvider {
}

// If heartbeatJSON is null, assume we're testing.

const baseURL = await Settings.get("primaryBaseURL");
const address = this.extractAddress(monitorJSON);
const hasAddress = address !== "" && address !== monitorJSON.hostname;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why != monitorJSON.hostname?
As far as I can remeber, none of the other monitors do that => lets also not do this, unless there is a reason for this.

If there is no particular reason, please inline hasAddress.
If there is, please add a comment why.


if (heartbeatJSON == null) {
let discordtestdata = {
const discordtestdata = {
username: discordDisplayName,
content: msg,
};
Expand All @@ -33,84 +39,89 @@ class Discord extends NotificationProvider {
return okMsg;
}

const embedFields = [
{
name: "Service Name",
value: monitorJSON.name,
},
...(hasAddress ? [{
name: "Service URL",
value: address
}] : []),
{
name: `Time (${heartbeatJSON.timezone})`,
value: heartbeatJSON.localDateTime,
},
{
name: "Error",
value: msg,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5181 (comment)

Suggested change
value: msg,
value: heartbeatJSON["msg"] ?? msg,

},
];

const components = [
{
type: 1, // Action Row
components: [
baseURL && {
type: 2, // Button
style: 5, // Link Button,
label: "Visit Uptime Kuma",
url: baseURL + getMonitorRelativeURL(monitorJSON.id)
},
hasAddress && {
type: 2, // Button
style: 5, // Link Button,
label: "Visit Service URL",
url: address
}
].filter(Boolean) // remove invalid data
}
];

// If heartbeatJSON is not null, we go into the normal alerting loop.
if (heartbeatJSON["status"] === DOWN) {
let discorddowndata = {
if (heartbeatJSON.status === DOWN) {
const discorddowndata = {
username: discordDisplayName,
content: notification.discordPrefixMessage || "",
embeds: [{
title: "❌ Your service " + monitorJSON["name"] + " went down. ❌",
title: `❌ Your service ${monitorJSON.name} went down. ❌`,
color: 16711680,
timestamp: heartbeatJSON["time"],
fields: [
{
name: "Service Name",
value: monitorJSON["name"],
},
{
name: monitorJSON["type"] === "push" ? "Service Type" : "Service URL",
value: this.extractAddress(monitorJSON),
},
{
name: `Time (${heartbeatJSON["timezone"]})`,
value: heartbeatJSON["localDateTime"],
},
{
name: "Error",
value: heartbeatJSON["msg"] == null ? "N/A" : heartbeatJSON["msg"],
},
],
timestamp: heartbeatJSON.time,
fields: embedFields,
}],
components: components,
};

if (notification.discordChannelType === "createNewForumPost") {
discorddowndata.thread_name = notification.postName;
}
if (notification.discordPrefixMessage) {
discorddowndata.content = notification.discordPrefixMessage;
}

await axios.post(webhookUrl.toString(), discorddowndata);
return okMsg;
}

} else if (heartbeatJSON["status"] === UP) {
let discordupdata = {
if (heartbeatJSON.status === UP) {
const discordupdata = {
username: discordDisplayName,
content: notification.discordPrefixMessage || "",
embeds: [{
title: "✅ Your service " + monitorJSON["name"] + " is up! ✅",
title: `✅ Your service ${monitorJSON.name} is up! ✅`,
color: 65280,
timestamp: heartbeatJSON["time"],
fields: [
{
name: "Service Name",
value: monitorJSON["name"],
},
{
name: monitorJSON["type"] === "push" ? "Service Type" : "Service URL",
value: this.extractAddress(monitorJSON),
},
{
name: `Time (${heartbeatJSON["timezone"]})`,
value: heartbeatJSON["localDateTime"],
},
{
name: "Ping",
value: heartbeatJSON["ping"] == null ? "N/A" : heartbeatJSON["ping"] + " ms",
},
],
timestamp: heartbeatJSON.time,
fields: embedFields,
}],
components: components,
};

if (notification.discordChannelType === "createNewForumPost") {
discordupdata.thread_name = notification.postName;
}

if (notification.discordPrefixMessage) {
discordupdata.content = notification.discordPrefixMessage;
}

await axios.post(webhookUrl.toString(), discordupdata);
return okMsg;
}
} catch (error) {
console.log(error);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is not already done so, it should be logged for all and using our log.error facities.
If this is the case, that would be for another PR.

Suggested change
console.log(error);

this.throwGeneralAxiosError(error);
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/components/notifications/Discord.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
<input id="discord-webhook-url" v-model="$parent.notification.discordWebhookUrl" type="text" class="form-control" required autocomplete="false">
<div class="form-text">
{{ $t("wayToGetDiscordURL") }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add a line break here, to prevent those lines being mushed together (think this adresses It's a little cursed from #5181 (comment))

Suggested change
{{ $t("wayToGetDiscordURL") }}
{{ $t("wayToGetDiscordURL") }}<br>

<i18n-t keypath="wayToGetDiscordURLEnhanced">
<a
href="https://discord.com/developers/docs/resources/webhook#create-webhook"
target="_blank"
>{{ $t("withADiscordBot") }}</a>
Comment on lines +8 to +11
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be simpler for our translators

Suggested change
<a
href="https://discord.com/developers/docs/resources/webhook#create-webhook"
target="_blank"
>{{ $t("withADiscordBot") }}</a>
<template #webhookWithADiscordBotAPI>
<a
href="https://discord.com/developers/docs/resources/webhook#create-webhook"
target="_blank"
>{{ $t("webhookWithADiscordBotAPI") }}</a>
</template>

</i18n-t>
</div>
</div>

Expand Down
2 changes: 2 additions & 0 deletions src/lang/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,8 @@
"smtpBCC": "BCC",
"Discord Webhook URL": "Discord Webhook URL",
"wayToGetDiscordURL": "You can get this by going to Server Settings -> Integrations -> View Webhooks -> New Webhook",
"wayToGetDiscordURLEnhanced": "Using the Discord API, you may also create the webhook {0} for button support.",
"withADiscordBot": "with a bot",
Comment on lines +544 to +545
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This word order reads a bit nicer in my mind. What do you think?

Suggested change
"wayToGetDiscordURLEnhanced": "Using the Discord API, you may also create the webhook {0} for button support.",
"withADiscordBot": "with a bot",
"wayToGetDiscordURLEnhanced": "For button support, you need to create create {webhookWithADiscordBotAPI}.",
"webhookWithADiscordBotAPI": "the webhook with a bot using the Discord API",

"Bot Display Name": "Bot Display Name",
"Prefix Custom Message": "Prefix Custom Message",
"Hello @everyone is...": "Hello {'@'}everyone is…",
Expand Down
Loading