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

[Discord] Service URL should only be included if defined #3327

Open
2 tasks done
OnCloud125252 opened this issue Jun 28, 2023 · 2 comments · May be fixed by #4495
Open
2 tasks done

[Discord] Service URL should only be included if defined #3327

OnCloud125252 opened this issue Jun 28, 2023 · 2 comments · May be fixed by #4495
Labels
area:notifications Everything related to notifications feature-request Request for new features to be added good first issue Good for newcomers type:enhance-existing feature wants to enhance existing monitor

Comments

@OnCloud125252
Copy link

⚠️ Please verify that this bug has NOT been raised before.

  • I checked and didn't find similar issue

🛡️ Security Policy

Description

This is what I received from the Discord notification webhook when one of the child services of a group I created went down. It display a section Service URL with a value https://. I don't think this section is necessary since it's a group and there's no related configuration in the settings.

Screenshot_2023-06-28-16-07-47-14_572064f74bd5f9fa804b05334aa4f912


The similar situation happened when the service is up. The section Service URL display https://. And the Ping section display N/A.

Screenshot_2023-06-28-16-29-26-92_572064f74bd5f9fa804b05334aa4f912

👟 Reproduction steps

Set up a Discord notification and enable it in a group.

👀 Expected behavior

  • For both Up and Down notification
    • The Service URL section should maintain empty since there's nothing to display with the URL.
    • Alternatively, I suggest it to display a list of the child services that are unreachable.
  • For Up notification
    • The Ping section should be remove or replace with other informations.

😓 Actual Behavior

The notification display unnecessary informations.

  • For both Up and Down notification
    • The section Service URL display a value https://.
  • For Up notification
    • The section Ping display a value N/A.

🐻 Uptime-Kuma Version

1.22.0

💻 Operating System and Arch

Ubuntu 22.04.2

🌐 Browser

No browser, it happens in Discord.

🐋 Docker Version

No response

🟩 NodeJS Version

16.20.1

📝 Relevant log output

No response

@OnCloud125252 OnCloud125252 added the bug Something isn't working label Jun 28, 2023
@CommanderStorm
Copy link
Collaborator

what kind of monitor are you using?
Could this be another symptom of the issue discussed in #3274

@OnCloud125252
Copy link
Author

@CommanderStorm

  1. In the latest release (1.22.0) there's a new monitor type named Group and it is supposed to monitor the child monitors, not the URL. So there shouldn't be a section named Service URL in the notification.
  2. I've seen that issue, I believe that my issue is different from the one you provided.

@chakflying chakflying changed the title Notification for group contains service url [Discord] Service URL should only be included if defined Dec 4, 2023
@chakflying chakflying added feature-request Request for new features to be added good first issue Good for newcomers and removed bug Something isn't working labels Dec 4, 2023
@CommanderStorm CommanderStorm added the area:notifications Everything related to notifications label Dec 5, 2023
@CommanderStorm CommanderStorm added the type:enhance-existing feature wants to enhance existing monitor label Dec 14, 2023
@NihadBadalov NihadBadalov linked a pull request Feb 15, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:notifications Everything related to notifications feature-request Request for new features to be added good first issue Good for newcomers type:enhance-existing feature wants to enhance existing monitor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants