Fix wrong end_time #946
Fix wrong end_time #946
Conversation
/.. also, attempt to clean up/document event types, and event type validation in the API.
hmpf
force-pushed
the
fix-wrong-end-time
branch
from
d6468ba
to
95a3ba7
Compare
|
| @@ -439,17 +462,36 @@ def tags(self): | |||
| def incident_relations(self): | |||
| return IncidentRelation.objects.filter(Q(incident1=self) | Q(incident2=self)) | |||
|
|
|||
| def all_opening_events(self): | |||
| open_events = (Event.Type.INCIDENT_START, Event.Type.REOPEN) | |||
There was a problem hiding this comment.
| open_events = (Event.Type.INCIDENT_START, Event.Type.REOPEN) | |
| open_events = Event.OPENING_TYPES |
| return self.events.filter(type__in=open_events).order_by("timestamp") | ||
|
|
||
| def all_reopen_events(self): | ||
| return self.events.filter(typen=Event.Type.REOPEN).order_by("timestamp") |
There was a problem hiding this comment.
| return self.events.filter(typen=Event.Type.REOPEN).order_by("timestamp") | |
| return self.events.filter(type=Event.Type.REOPEN).order_by("timestamp") |
| return self.events.filter(typen=Event.Type.REOPEN).order_by("timestamp") | ||
|
|
||
| def all_closing_events(self): | ||
| close_events = (Event.Type.CLOSE, Event.Type.INCIDENT_END) |
There was a problem hiding this comment.
| close_events = (Event.Type.CLOSE, Event.Type.INCIDENT_END) | |
| close_events = Event.CLOSING_TYPES |
| if self.stateless_event: | ||
| # Weird, stateless event without stateless end_time, fix | ||
| self.end_time = None | ||
| self.save() | ||
| LOG.warn("Mismatch between self %s end_time and event type: set stateless", self.pk) | ||
| return True |
There was a problem hiding this comment.
Seems like a normalization issue? Two ways of indicating that an event is stateless?
| def event_already_exists(self, event_type): | ||
| return self.events.filter(type=event_type).exists() | ||
|
|
||
| def repair_end_time(self) -> Optional[bool]: |
There was a problem hiding this comment.
Make sure that you have prefetched all events here, because otherwise thie function may cause a bunch of round trips to the db
| last_close_event = close_events.last() | ||
| if not reopen_event or reopen_event.timestamp < last_close_event.timestamp: |
There was a problem hiding this comment.
can last_close_event be None? Perhaps if there is a reopen_event there also always should be a close event. But you know, "should"...
| incident.repair_end_time() | ||
| abort_due_to_too_many_events(incident, event_type) |
There was a problem hiding this comment.
This is where we would end up in our case. There already is an END event, but the incident is reported open, so we fire another END event. The incident is repaired, so the end_time is correctly set (closing the incident), but we still get a 400 error back like we did something wrong. We're being gaslighted here: "I didn't make a mistake and just fixed it, you made a mistake!" 😂
perhaps look a the return value of the repair?
| if event_type in Event.CLOSING_TYPES and not incident.open: | ||
| self._abort_due_to_type_validation_error("The incident is already closed.") | ||
| if event_type == Event.Type.REOPEN and incident.open: | ||
| self._abort_due_to_type_validation_error("The incident is already open.") |
There was a problem hiding this comment.
Are there situations here where we want to repair the incident here as well?
Hopefylly closes #935
Also, attempt to clean up/document event types, and event type validation in the API.