Skip to content

fix(topologies): skip deleted incidents when looking up application incidents - #6733

Merged
shahargl merged 1 commit into
keephq:mainfrom
Georgefifth:fix/topology-deleted-incident-lookup
Aug 23, 2026
Merged

fix(topologies): skip deleted incidents when looking up application incidents#6733
shahargl merged 1 commit into
keephq:mainfrom
Georgefifth:fix/topology-deleted-incident-lookup

Conversation

@Georgefifth

Copy link
Copy Markdown
Contributor

Fixes #6732

Problem

Deleting a topology incident only marks it deleted — the row keeps its incident_application link. _get_application_based_incident did not filter on status, so the next alert for that application reused the deleted incident instead of creating a new one.

Solution

Exclude deleted incidents from the lookup: Incident.status != IncidentStatus.DELETED.value (same pattern already used in keep/api/core/db.py). One-line change, no behavior change for firing/resolved/acknowledged incidents.

Result

  • Added regression test test_get_application_based_incident_skips_deleted_incident: fails on the old query (RED), passes with the filter (GREEN)
  • tests/test_topology.py: 13/13 passed
  • tests/test_incidents.py: 25/25 passed (no regressions)
  • ruff check clean on both touched files

…ncidents

Deleting an incident only marks it deleted; the row keeps its
incident_application link. The lookup query did not filter on
status, so a new alert for the same application reused the deleted
incident instead of creating a new one.

Exclude deleted incidents from the lookup and add a regression test
(RED on the old query, green with the filter).

Fixes keephq#6732
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. Bug Something isn't working labels Aug 23, 2026
@CLAassistant

CLAassistant commented Aug 23, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@shahargl shahargl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 23, 2026
@shahargl
shahargl merged commit e3d2b79 into keephq:main Aug 23, 2026
5 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

🌟 Fantastic work @Georgefifth! Your very first PR to keep has been merged! 🎉🥳

You've just taken your first step into open-source, and we couldn't be happier to have you onboard. 🙌
If you're feeling adventurous, why not dive into another issue and keep contributing? The community would love to see more from you! 🚀

For any support, feel free to reach out on the community: https://slack.keephq.dev. Happy coding! 👩‍💻👨‍💻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Topology incident remains linked after incident deletion

3 participants