Skip to content

Conversation

SOwOphie
Copy link

@SOwOphie SOwOphie commented Dec 30, 2024

As noted in #3798, it would be great to have the alarm notification include the full monitor path. This was done for Mattermost in #3801, but not for all the other notification providers. This commit replaces all other trivial occurences of monitorJSON.name with monitorJSON.pathName.

I only really care about the Discord notification providers, but I also changed all the other low hanging fruits I could find. Only Discord is personally tested by me as I do not have access to the other providers. If you consider this too sketchy to merge, I will gladly drop the changes to the other providers.


⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Fixes #3798

Type of change

Please delete any options that are not relevant.

  • Other

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

image

…n providers (where applicable)

As noted in louislam#3798, it would be great to have the alarm notification include
the full monitor path.  This was done for Mattermost in louislam#3801, but not for
the other notification providers.  This commit replaces all other trivial
occurences of monitorJSON.name with monitorJSON.pathName.
@CommanderStorm CommanderStorm changed the base branch from master to 2.1.X January 24, 2025 19:05
@CommanderStorm CommanderStorm added this to the 2.1.0 milestone Jan 24, 2025
@CommanderStorm CommanderStorm added the pr:depends on other pending other things to be done first label Jan 24, 2025
@CommanderStorm CommanderStorm changed the title Replace monitorJSON.name with monitorJSON.pathName in all notification providers (where applicable) feat: replace monitorJSON.name with monitorJSON.pathName to display the monitors path, not just the root Jan 24, 2025
@CommanderStorm
Copy link
Collaborator

@SOwOphie Short headsup: @II-EMC noticed in #5760 that pathName does not contain the name anymore. Could you have a look if this affects your change as well?

Not shure what caused this regression or if this was intentional.

@CommanderStorm CommanderStorm modified the milestones: 2.1.0, 2.0.0-beta.3 May 17, 2025
@CommanderStorm CommanderStorm changed the base branch from 2.1.X to master May 17, 2025 22:19
@CommanderStorm CommanderStorm changed the base branch from master to 2.1.X May 17, 2025 22:21
@CommanderStorm CommanderStorm modified the milestones: 2.0.0-beta.3, 2.1.0 May 17, 2025
@SOwOphie
Copy link
Author

Due to personal health issues, I see myself unable to deal with this in a reasonable time frame. If anybody else wants to take this up, please be my guest. Sorry for wasting your time.

@SOwOphie SOwOphie closed this May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:depends on other pending other things to be done first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Full monitor path in notifications

2 participants