fix(discord): hide empty Service URL and N/A Ping fields#6561
Conversation
- Only show Service URL field when extractAddress returns a non-empty value - Only show Ping field when ping value is not null - This fixes unnecessary 'https://' and 'N/A' values showing for groups Fixes louislam#3327 Contribution by Gittensor, see my contribution statistics at https://gittensor.io/miners/details?githubId=101010297
CommanderStorm
added
the
pr:please address review comments
CommanderStorm
removed
the
pr:please address review comments
There was a problem hiding this comment.
Pull request overview
This PR improves Discord notifications by conditionally hiding fields that contain no meaningful information. The changes prevent displaying empty Service URLs (like "https://" for monitor groups) and "N/A" Ping values when ping data is unavailable.
Key Changes:
- Extract address value once into a variable for reuse
- Add conditional check to only show Service URL field when address has a value
- Add conditional check to only show Ping field when ping is not null
| } | ||
|
|
||
| // If heartbeatJSON is not null, we go into the normal alerting loop. | ||
| let addess = this.extractAddress(monitorJSON); |
There was a problem hiding this comment.
The variable name "addess" is misspelled. It should be "address" (missing 'r').
| ...((!notification.disableUrl && addess) ? [{ | ||
| name: monitorJSON["type"] === "push" ? "Service Type" : "Service URL", | ||
| value: this.extractAddress(monitorJSON), | ||
| value: addess, |
There was a problem hiding this comment.
The variable name "addess" is misspelled. It should be "address" (missing 'r'). This affects all references throughout the function.
| ...((!notification.disableUrl && addess) ? [{ | ||
| name: monitorJSON["type"] === "push" ? "Service Type" : "Service URL", | ||
| value: this.extractAddress(monitorJSON), | ||
| value: addess, |
There was a problem hiding this comment.
The variable name "addess" is misspelled. It should be "address" (missing 'r'). This affects all references throughout the function.
Summary
extractAddressreturns a non-empty valueThis fixes the issue where Discord notifications display unnecessary information:
Service URL: https://for groups (which have no actual URL)Ping: N/Awhen ping value is not availableChanges
Modified
server/notification-providers/discord.jsto conditionally include these fields only when they have meaningful values.Test plan
Fixes #3327
Contribution by Gittensor, see my contribution statistics at https://gittensor.io/miners/details?githubId=101010297