From 5f9ebb178ccfc330166b59628b87e42b4efc8cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Wed, 19 Dec 2018 21:57:48 +0200 Subject: [PATCH] Rename "Check.get_alert_after" to a now more fitting "Check.going_down_after" --- hc/api/management/commands/sendalerts.py | 9 ++-- hc/api/models.py | 17 ++++--- ...fter.py => test_check_going_down_after.py} | 14 +++--- hc/api/tests/test_ping.py | 3 +- hc/api/tests/test_sendalerts.py | 44 ++++++++++--------- hc/front/tests/test_update_timeout.py | 7 +-- hc/front/views.py | 4 +- 7 files changed, 53 insertions(+), 45 deletions(-) rename hc/api/tests/{test_check_alert_after.py => test_check_going_down_after.py} (76%) diff --git a/hc/api/management/commands/sendalerts.py b/hc/api/management/commands/sendalerts.py index d263044f..30e78656 100644 --- a/hc/api/management/commands/sendalerts.py +++ b/hc/api/management/commands/sendalerts.py @@ -96,23 +96,22 @@ class Command(BaseCommand): if not check.is_down(): # It is not down yet. Update alert_after - q.update(alert_after=check.get_alert_after()) + q.update(alert_after=check.going_down_after()) return True # Atomically update status - num_updated = q.update(status="down") + flip_time = check.going_down_after() + num_updated = q.update(alert_after=None, status="down") if num_updated != 1: # Nothing got updated: another worker process got there first. return True flip = Flip(owner=check) - flip.created = check.get_alert_after() + flip.created = flip_time flip.old_status = old_status flip.new_status = "down" flip.save() - check.status = "down" - check.save() return True def handle(self, use_threads=True, loop=True, *args, **options): diff --git a/hc/api/models.py b/hc/api/models.py index 07e75529..7e35d0f9 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -142,8 +142,13 @@ class Check(models.Model): if result != NEVER: return result - def get_alert_after(self): - """ Return the datetime when check potentially goes down. """ + def going_down_after(self): + """ Return the datetime when the check goes down. + + If the check is new or paused, and not currently running, return None. + If the check is already down, also return None. + + """ grace_start = self.get_grace_start() if grace_start is not None: @@ -155,11 +160,11 @@ class Check(models.Model): if self.status == "down": return True - alert_after = self.get_alert_after() - if alert_after is None: + down_after = self.going_down_after() + if down_after is None: return False - return timezone.now() >= alert_after + return timezone.now() >= down_after def get_status(self, now=None): """ Return current status for display. """ @@ -246,7 +251,7 @@ class Check(models.Model): self.status = new_status - self.alert_after = self.get_alert_after() + self.alert_after = self.going_down_after() self.n_pings = models.F("n_pings") + 1 self.has_confirmation_link = "confirm" in str(body).lower() self.save() diff --git a/hc/api/tests/test_check_alert_after.py b/hc/api/tests/test_check_going_down_after.py similarity index 76% rename from hc/api/tests/test_check_alert_after.py rename to hc/api/tests/test_check_going_down_after.py index c7d1dc3f..631dfb12 100644 --- a/hc/api/tests/test_check_alert_after.py +++ b/hc/api/tests/test_check_going_down_after.py @@ -9,20 +9,20 @@ class CheckModelTestCase(TestCase): def test_it_handles_new_check(self): check = Check() - self.assertEqual(check.get_alert_after(), None) + self.assertEqual(check.going_down_after(), None) self.assertFalse(check.is_down()) def test_it_handles_paused_check(self): - check = Check() + check = Check(status="paused") check.last_ping = timezone.now() - td(days=2) - self.assertEqual(check.get_alert_after(), None) + self.assertEqual(check.going_down_after(), None) self.assertFalse(check.is_down()) def test_it_handles_up(self): check = Check(status="up") check.last_ping = timezone.now() - td(hours=1) expected_aa = check.last_ping + td(days=1, hours=1) - self.assertEqual(check.get_alert_after(), expected_aa) + self.assertEqual(check.going_down_after(), expected_aa) self.assertFalse(check.is_down()) def test_it_handles_paused_then_started_check(self): @@ -30,18 +30,18 @@ class CheckModelTestCase(TestCase): check.last_start = timezone.now() - td(days=2) expected_aa = check.last_start + td(hours=1) - self.assertEqual(check.get_alert_after(), expected_aa) + self.assertEqual(check.going_down_after(), expected_aa) self.assertTrue(check.is_down()) def test_it_handles_down(self): check = Check(status="down") check.last_ping = timezone.now() - td(hours=1) - self.assertEqual(check.get_alert_after(), None) + self.assertEqual(check.going_down_after(), None) self.assertTrue(check.is_down()) def test_it_handles_down_then_started_check(self): check = Check(status="down") check.last_start = timezone.now() - td(minutes=10) - self.assertEqual(check.get_alert_after(), None) + self.assertEqual(check.going_down_after(), None) self.assertTrue(check.is_down()) diff --git a/hc/api/tests/test_ping.py b/hc/api/tests/test_ping.py index a3cb45f5..b9747a27 100644 --- a/hc/api/tests/test_ping.py +++ b/hc/api/tests/test_ping.py @@ -17,7 +17,8 @@ class PingTestCase(TestCase): self.check.refresh_from_db() self.assertEqual(self.check.status, "up") - self.assertEqual(self.check.alert_after, self.check.get_alert_after()) + expected_aa = self.check.last_ping + td(days=1, hours=1) + self.assertEqual(self.check.alert_after, expected_aa) ping = Ping.objects.latest("id") self.assertEqual(ping.scheme, "http") diff --git a/hc/api/tests/test_sendalerts.py b/hc/api/tests/test_sendalerts.py index dc568e63..4da2d559 100644 --- a/hc/api/tests/test_sendalerts.py +++ b/hc/api/tests/test_sendalerts.py @@ -1,4 +1,4 @@ -from datetime import timedelta +from datetime import timedelta as td from io import StringIO from mock import Mock, patch @@ -14,18 +14,20 @@ class SendAlertsTestCase(BaseTestCase): def test_it_handles_grace_period(self): check = Check(user=self.alice, status="up") # 1 day 30 minutes after ping the check is in grace period: - check.last_ping = now() - timedelta(days=1, minutes=30) - check.alert_after = check.get_alert_after() + check.last_ping = now() - td(days=1, minutes=30) + check.alert_after = check.last_ping + td(days=1, hours=1) check.save() Command().handle_going_down() + check.refresh_from_db() + self.assertEqual(check.status, "up") self.assertEqual(Flip.objects.count(), 0) def test_it_creates_a_flip_when_check_goes_down(self): check = Check(user=self.alice, status="up") - check.last_ping = now() - timedelta(days=2) - check.alert_after = check.get_alert_after() + check.last_ping = now() - td(days=2) + check.alert_after = check.last_ping + td(days=1, hours=1) check.save() result = Command().handle_going_down() @@ -36,17 +38,19 @@ class SendAlertsTestCase(BaseTestCase): # It should create a flip object flip = Flip.objects.get() self.assertEqual(flip.owner_id, check.id) + self.assertEqual(flip.created, check.alert_after) self.assertEqual(flip.new_status, "down") - # It should change stored status to "down" + # It should change stored status to "down", and clear out alert_after check.refresh_from_db() self.assertEqual(check.status, "down") + self.assertEqual(check.alert_after, None) @patch("hc.api.management.commands.sendalerts.notify_on_thread") def test_it_processes_flip(self, mock_notify): check = Check(user=self.alice, status="up") check.last_ping = now() - check.alert_after = check.get_alert_after() + check.alert_after = check.last_ping + td(days=1, hours=1) check.save() flip = Flip(owner=check, created=check.last_ping) @@ -69,7 +73,7 @@ class SendAlertsTestCase(BaseTestCase): @patch("hc.api.management.commands.sendalerts.notify_on_thread") def test_it_updates_alert_after(self, mock_notify): check = Check(user=self.alice, status="up") - check.last_ping = now() - timedelta(hours=1) + check.last_ping = now() - td(hours=1) check.alert_after = check.last_ping check.save() @@ -79,8 +83,9 @@ class SendAlertsTestCase(BaseTestCase): self.assertTrue(result) # alert_after should have been increased + expected_aa = check.last_ping + td(days=1, hours=1) check.refresh_from_db() - self.assertTrue(check.alert_after > check.last_ping) + self.assertEqual(check.alert_after, expected_aa) # a flip should have not been created self.assertEqual(Flip.objects.count(), 0) @@ -88,8 +93,8 @@ class SendAlertsTestCase(BaseTestCase): @patch("hc.api.management.commands.sendalerts.notify") def test_it_works_synchronously(self, mock_notify): check = Check(user=self.alice, status="up") - check.last_ping = now() - timedelta(days=2) - check.alert_after = check.get_alert_after() + check.last_ping = now() - td(days=2) + check.alert_after = check.last_ping + td(days=1, hours=1) check.save() call_command("sendalerts", loop=False, use_threads=False, @@ -99,12 +104,11 @@ class SendAlertsTestCase(BaseTestCase): self.assertTrue(mock_notify.called) def test_it_updates_owners_next_nag_date(self): - self.profile.nag_period = timedelta(hours=1) + self.profile.nag_period = td(hours=1) self.profile.save() check = Check(user=self.alice, status="down") - check.last_ping = now() - timedelta(days=2) - check.alert_after = check.get_alert_after() + check.last_ping = now() - td(days=2) check.save() flip = Flip(owner=check, created=check.last_ping) @@ -118,12 +122,11 @@ class SendAlertsTestCase(BaseTestCase): self.assertIsNotNone(self.profile.next_nag_date) def test_it_updates_members_next_nag_date(self): - self.bobs_profile.nag_period = timedelta(hours=1) + self.bobs_profile.nag_period = td(hours=1) self.bobs_profile.save() check = Check(user=self.alice, status="down") - check.last_ping = now() - timedelta(days=2) - check.alert_after = check.get_alert_after() + check.last_ping = now() - td(days=2) check.save() flip = Flip(owner=check, created=check.last_ping) @@ -137,14 +140,13 @@ class SendAlertsTestCase(BaseTestCase): self.assertIsNotNone(self.bobs_profile.next_nag_date) def test_it_does_not_touch_already_set_next_nag_dates(self): - original_nag_date = now() - timedelta(minutes=30) - self.profile.nag_period = timedelta(hours=1) + original_nag_date = now() - td(minutes=30) + self.profile.nag_period = td(hours=1) self.profile.next_nag_date = original_nag_date self.profile.save() check = Check(user=self.alice, status="down") - check.last_ping = now() - timedelta(days=2) - check.alert_after = check.get_alert_after() + check.last_ping = now() - td(days=2) check.save() flip = Flip(owner=check, created=check.last_ping) diff --git a/hc/front/tests/test_update_timeout.py b/hc/front/tests/test_update_timeout.py index 1cafac49..c8512f2d 100644 --- a/hc/front/tests/test_update_timeout.py +++ b/hc/front/tests/test_update_timeout.py @@ -1,7 +1,7 @@ from datetime import timedelta as td from django.utils import timezone -from hc.api.models import Flip, Check +from hc.api.models import Check from hc.test import BaseTestCase @@ -9,7 +9,7 @@ class UpdateTimeoutTestCase(BaseTestCase): def setUp(self): super(UpdateTimeoutTestCase, self).setUp() - self.check = Check(user=self.alice) + self.check = Check(user=self.alice, status="up") self.check.last_ping = timezone.now() self.check.save() @@ -28,7 +28,8 @@ class UpdateTimeoutTestCase(BaseTestCase): self.assertEqual(self.check.grace.total_seconds(), 60) # alert_after should be updated too - self.assertEqual(self.check.alert_after, self.check.get_alert_after()) + expected_aa = self.check.last_ping + td(seconds=3600 + 60) + self.assertEqual(self.check.alert_after, expected_aa) def test_it_does_not_update_status(self): self.check.last_ping = timezone.now() - td(days=2) diff --git a/hc/front/views.py b/hc/front/views.py index 0b639e3e..3d97952f 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -18,7 +18,7 @@ from django.utils.crypto import get_random_string from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_POST from hc.api.models import (DEFAULT_GRACE, DEFAULT_TIMEOUT, Channel, Check, - Flip, Ping, Notification) + Ping, Notification) from hc.api.transports import Telegram from hc.front.forms import (AddWebhookForm, NameTagsForm, TimeoutForm, AddUrlForm, AddEmailForm, @@ -297,7 +297,7 @@ def update_timeout(request, code): check.tz = form.cleaned_data["tz"] check.grace = td(minutes=form.cleaned_data["grace"]) - check.alert_after = check.get_alert_after() + check.alert_after = check.going_down_after() check.save() if "/details/" in request.META.get("HTTP_REFERER", ""):