feat: implement very crude and bare-bones RSS feed #5047
Conversation
| heartbeats.forEach(heartbeat => { | ||
| feed.addItem({ | ||
| title: `${heartbeat.name} is down`, | ||
| description: `${heartbeat.name} has been down since ${heartbeat.time}`, |
There was a problem hiding this comment.
Not quite true.
That is just the last heartbeat that failed.
That description however can be misinterpreted as the first DOWN heartbeat (=> the total downtime for said heatbeat).
Also: I don't think that it has to be down, right? (we also have MAINTENANCE, UP and Pending as statuses, but I would need to look up if they can be heartbeats)
There was a problem hiding this comment.
I found this code in StatusPage.vue that interprets the status field from the database:
let status = STATUS_PAGE_ALL_UP;
let hasUp = false;
for (let id in this.$root.publicLastHeartbeatList) {
let beat = this.$root.publicLastHeartbeatList[id];
if (beat.status === MAINTENANCE) {
return STATUS_PAGE_MAINTENANCE;
} else if (beat.status === UP) {
hasUp = true;
} else {
status = STATUS_PAGE_PARTIAL_DOWN;
}
}
if (! hasUp) {
status = STATUS_PAGE_ALL_DOWN;
}
return status;I'll do something similar for the RSS feed 👍
There was a problem hiding this comment.
To your questions,
That is just the last heartbeat that failed.
That description however can be misinterpreted as the first DOWN heartbeat (=> the total downtime for said heatbeat).
in getRSSPageData, I'm filtering for the last heartbeat of monitor_id=?. If that heartbeat is falsy, its added to the RSS feed. When the monitor is UP again, the RSS feed won't publish it anymore. I think this is the desired behaviour.
There was a problem hiding this comment.
Filtering for DOWN sounds fine (I don't use RSS-Feeds, your call).
Have you tested that a falsy heartbeat means what you think it does?
Only the following values are falsy: https://developer.mozilla.org/en-US/docs/Glossary/Falsy
=> you are not filtering for the things you think you are..
There was a problem hiding this comment.
Great call mate. I've not worked with js for many years, so I'm a little bit rusty on these quirks 👍
CommanderStorm
added
area:status-page
|
Hello again! I think I've addressed your comments @CommanderStorm. What's hindering us from getting this PR merged? 👍 |
|
Blocking merging is Amongst others that my last review #5047 (comment) is not addressed as far as I can tell.. You are still filtering for truthy-ness and thus likely displaying |
|
@CommanderStorm that's in my opinion addressed in commit 35349a , where I explicitly look for DOWN beats. What do you mean with "Amongst others"? Are there more unanswered comments? |
There was a problem hiding this comment.
Sorry, I could not do a proper review yesterday because of a terrible internet connection in Shanghai (firewall and stuffs you know).
(I flew onto vacation to Tokio)
LGTM now, I overlooked the reason why this is not filtered below.
Merging with junior-approval
CommanderStorm
removed
the
pr:please address review comments
|
Thank you @CommanderStorm for your guidance in this PR! Happy vacation! |
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]:
Description
Fixes #1673.
This PR adds RSS capabilities to status-pages without extra configuration.
It's implemented as a custom route: append
/rssto any status URL to get the RSS feed.I've checked out the discussions in #2129 and #1673.
This PR adds a new server-side dependency
feed.Right now, the RSS feed looks like this:
Right now, the RSS feed is missing this information that exists on the regular status page:
Type of change
Checklist
Screenshots (if any)
Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.