Use correct filterwrapper for pm #1667
Conversation
|
Test results 4 files 528 suites 10m 36s ⏱️ Results for commit 94aa642. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1667 +/- ##
==========================================
+ Coverage 79.70% 79.74% +0.03%
==========================================
Files 133 133
Lines 6174 6196 +22
==========================================
+ Hits 4921 4941 +20
- Misses 1253 1255 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| def current(self): | ||
| return self.active_at_time(timezone.now()) | ||
|
|
||
| def active_at_time(self, time: Optional[datetime] = None): |
There was a problem hiding this comment.
Oh, that one got lost during rebasing 😅
| return super().incident_fits(incident) | ||
|
|
||
| def event_fits(self, event: Event) -> bool: | ||
| is_ongoing = self.is_ongoing(self.timestamp) |
There was a problem hiding this comment.
Could you add a docstring on why you're testing is_ongoing with the PM timestamp? Because from my understanding we only need to test if the time the event happened the PM was ongoing
|
|
||
| def event_covered_by_planned_maintenance(event: Event, pm_tasks: Optional[PlannedMaintenanceQuerySet] = None) -> bool: | ||
| def event_covered_by_planned_maintenance( | ||
| event: Event, pm_tasks: Optional[PlannedMaintenanceQuerySet] = None, timestamp: Optional[datetime] = None |
There was a problem hiding this comment.
I'm confused by the timestamp here - it is optional all the way until is_ongoing, where it is required, but we never set it if it is None
There was a problem hiding this comment.
And I don't fully understand what the timestamp is user for
Scope and purpose
Ensures that notificationprofile and pm tasks use filters the same way.
Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Argus can be found in the
Development docs.
[ ] Added a changelog fragment for towncrier[ ] Added/changed documentation, including updates to the user manual if feature flow or UI is considerably changed[ ] If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done[ ] If this results in changes in the UI: Added screenshots of the before and after[ ] If this results in changes to the database model: Updated the ER diagram