-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
feat: implement incident history #6469
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
feat: implement incident history #6469
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements an incident timeline/history feature for status pages, enabling multiple pinned/active incidents and providing basic management capabilities (edit, resolve, delete). The implementation changes the existing single-incident model to support multiple active incidents, adds a paginated incident history view, and introduces new UI components for incident management.
Key Changes:
- Changed from single incident (
findOne) to multiple active incidents (find) with pinning support - Added paginated incident history with progressive loading via "Load More" button
- Implemented per-status-page incident management through both inline editing and modal interfaces
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/StatusPage.vue | Added incident history display with pagination, multiple active incidents support, inline editing for active incidents, and new methods for incident management |
| src/components/IncidentHistory.vue | New component displaying incident list with edit/delete/resolve actions for authenticated users |
| src/components/IncidentManageModal.vue | New modal component for editing incidents from the history section |
| src/lang/en.json | Added translation keys for incident history UI elements and validation messages |
| server/socket-handlers/status-page-socket-handler.js | Added socket handlers for incident history retrieval, editing, deletion, and resolution with input validation |
| server/routers/status-page-router.js | Added REST API endpoint for public incident history access with pagination |
| server/model/status_page.js | Changed incident retrieval from single to multiple active pinned incidents |
| server/model/incident.js | Added resolve() method and toJSON() method for admin data serialization |
Comments suppressed due to low confidence (1)
src/pages/StatusPage.vue:211
- The dropdown button has a duplicate ID "dropdownMenuButton1" which appears on lines 208 and 254. HTML IDs must be unique within a document. This can cause accessibility issues and incorrect behavior with Bootstrap dropdowns.
Use unique IDs for each dropdown, such as:
- Line 208:
id="dropdownMenuButton-new" - Line 254:
id="dropdownMenuButton-edit"
And update the corresponding aria-labelledby attributes on lines 211 and 257.
<button id="dropdownMenuButton1" class="btn btn-secondary dropdown-toggle" type="button" data-bs-toggle="dropdown" aria-expanded="false">
{{ $t("Style") }}: {{ $t(incident.style) }}
</button>
<ul class="dropdown-menu" aria-labelledby="dropdownMenuButton1">
|
converting to draft to double check copilots review |
update incorrect translation add resolved translation use translation for unk remove duplicate load fix state management for cancelling edit propagate error if unable to load incident history (unlikely) fix xss
5e1c1d8 to
71f32ae
Compare
|
@aluminyoom Great work! However, I noticed that this PR only addresses the issue with displaying multiple incidents. In #862, the author previously suggested an approach commonly used by other status page services — not a literal visual timeline, but rather a series of incident update messages (start, intermediate updates, resolution). Their timeline mockup was admittedly bulky, but the idea behind message-based incident updates is a well-established pattern in many status pages. It might be worth considering this direction as well. |
|
Thanks, @demidovegor This PR can definitely go in that direction, and I agree and would have done so if not for the DB schema changes it would require. The goal for this PR is to have a barebones Incident History that utilizes the existing schema while making it possible to extend in the future so other people can build on top of it (in your case, supporting the intermittent updates) Can definitely introduce a stacked pr on top of this change, with the schema changes and the in-betweeners: start, intermediate updates, resolution. |
fix dry violations and revert nit move to util.ts
2a360ef to
1057cc9
Compare
CommanderStorm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I have been super buisy over the holidays.
Code looks good and is well structured 🎉
I have some minor comments, but nothing major.
I have two blocking issues:
- this is a constant 10 incidents on each status page that uses incidents, which is fairly long. Can we move the exact current UI into a modal (maybe better? not sure?) or should we shorten this to something like 2? 🤔
- I think there is a bug that the current active incidents are also shown under "Past incidents"
| toJSON() { | ||
| return { | ||
| ...this.toPublicJSON(), | ||
| status_page_id: this.status_page_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't consider this quite internal, or am I missing something? 🤔
Can't this just be added to toPublicJSON?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely trivial yea, reasoning behind this is to expose the only needed data for public endpoints. since status_page_id doesn't really get consumed on the public facing side.
we can add it to toPublicJSON but it would be unused for public endpoints.
edit: formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets simplify this then to just include it up in toPublicJSON.
server/model/status_page.js
Outdated
| static async getIncidentHistory(statusPageId, page, isPublic = true) { | ||
| const offset = (page - 1) * INCIDENT_PAGE_SIZE; | ||
|
|
||
| const incidents = await R.find("incident", | ||
| " status_page_id = ? ORDER BY created_date DESC LIMIT ? OFFSET ? ", | ||
| [ statusPageId, INCIDENT_PAGE_SIZE, offset ] | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of offset based, can we make it created_date based?
Here is a blogpost explaining the rationale and why limit/offset is usually buggy.
https://cedardb.com/blog/pagination/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats a very interesting and great read. i'll go ahead and do that, thanks!
| } | ||
| }); | ||
|
|
||
| socket.on("getIncidentHistory", async (slug, page, callback) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getIncidentHistory and getPublicIncidentHistory only differ in the one checking for login and one true/false.
Can we merge them into one where true/false is set based on the login status?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure!
src/pages/StatusPage.vue
Outdated
| </div> | ||
|
|
||
| <!-- Past Incidents --> | ||
| <div class="past-incidents-section mb-4"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite know where this bug is from, but this section also includes the currently active, not just the past incidents.
Can we only have the past incidents here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yea, my reasoning for this one is to include ongoing incidents under past incidents as well.
for me it seemed logical to put it there as the "past incidents" view is a historical timeline of all incidents, and having a pinned incident should still show on history don't you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for reference the "bug" is this located here. we don't filter out by status and return the incidents regardless of state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well if they are "Past", that means that they are sort of historical, right?
I think the currently actively pinned are not historical.
I would not include them down there.
Alternatively, we can change the label, but I think most people would expect it to not be there.
@demidovegor maybe you can give your oppinion on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc'ing the original issue creator as well for input: @14wkinnersley
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see this go both ways. I would prefer the "Past Incidents" to show actual past and not a duplicate of the current ongoing incidents. I did initially find that confusing when I was testing, but it didn't bother me having it shown there since it was designed in a way you can clearly differentiate which are active incidents and which are old. Id prefer it not to be duplicated though
|
Also could you add some e2e tests for this functionality? |
| </div> | ||
| </div> | ||
|
|
||
| <div v-else-if="incidents.length === 0" class="text-center py-4 text-muted"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not show the entire history in this case, as I assume that the vast majority of our users don't use this feature (communicating status acively is sadly less common I think).
-> Should we hide it? what do you think is better overall?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for minimalism (my bias), i'd prefer to hide it. though keeping it shows that incident history is a thing and shows reliability.
i guess keeping it an option would be better (default is to not show history)? but that would be touching the other components already. for this case i'd keep it then implement the option later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for minimalism (my bias), i'd prefer to hide it
Then lets do that 😉
CommanderStorm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, we can merge when you want (the PR is currently still a draft)
|
okie dokie undrafted |
|
will fix |
Head branch was pushed to by a user without write access
|
converting to draft (have to resolve e2e failures) |
|
@aluminyoom congrats on your first contribution to Uptime Kuma! 🐻 |





📋 Overview
What problem does this pull request address?
This PR adds an Incident Timeline/History feature to status pages with basic management (EDIT/RESOLVE/DELETE). This implementation is designed to be minimal to allow building upon so we don't introduce further feature bloat. Additionally, this PR enables multiple pinned/active incidents.
What features or functionality does this pull request introduce or enhance?
This PR allows a status page to have multiple pinned/active incidents as opposed to previously one. Basically change (findOne -> find)
This PR shows the past incidents grouped by date (if existing) reusing the existing incidents table for now.
Incident Management is PER status page NOT global. The past incidents page loads progressively via the Load More button.
The management side of past incidents are in the incident history itself, it feels native as incidents are managed per status page.
Re: Pinning and unpinning
nit(unrelated/noise): changed slug is not found -> slug not found(removed in 1395388)nit(maintainability):(addressed 2a360ef)const INCIDENT_PAGE_SIZE = 10;move toutil.tsnit(unrelated): running
npm run tscproduced unrelated changes to my change inutil.ts, not sure if i should keep it.This PR used an LLM to generate and format JSDoc comments.
This PR used an LLM to generate review comments and some of those comments have been accepted via Copilot.
🛠️ Type of change
📄 Checklist
📷 Screenshots or Visual Changes