feat: add merge incidents endpoint [dnm] #2259
Conversation
Signed-off-by: Kirill Chernakov <[email protected]>
…q/keep into feature/2042-split-merge-incidents
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Kiryous
changed the base branch from
main
to
feature/2042-split-merge-incidents-feature
Kiryous
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/2042-split-merge-incidents-feature #2259 +/- ##
=============================================================================
Coverage ? 31.61%
=============================================================================
Files ? 63
Lines ? 6674
Branches ? 0
=============================================================================
Hits ? 2110
Misses ? 4564
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Kiryous
changed the title
keep/api/core/db.py
Outdated
| ) | ||
|
|
||
| try: | ||
| add_alerts_to_incident( |
There was a problem hiding this comment.
I would probably suggest moving add_alerts_to_incident close to remove_alerts_to_incident_by_incident_id. So it will remove and immediately add... Per-incident. In this case exception in add_alerts_to_incident will damage only one Incident, not all.
| updated_incident_dto = IncidentDto.from_db_incident(updated_incident) | ||
| return updated_incident_dto | ||
| except DestinationIncidentNotFound as e: | ||
| raise HTTPException(status_code=400, detail=str(e)) |
There was a problem hiding this comment.
You could actually just inherit DestinationIncidentNotFound from HTTPException and call merge_incidents_to_id without try/except here to let fastapi error handling to the magic. Not critical
There was a problem hiding this comment.
just inherit DestinationIncidentNotFound from HTTPException
yes, but it will link the service layer with view layer. e.g. db.py will be refactored to incidents_service some day and this method could be used in non-routes methods, therefore I propose to keep it not linked.
Kiryous
merged commit 2dee447
into
feature/2042-split-merge-incidents-feature
Closes #2258
This PR targeting feature branch and a part of the whole feature.
TODO: