From 6f5a22fd9874e70b246ebc8a4acb847af5052a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Thu, 26 Aug 2021 10:35:05 +0300 Subject: [PATCH] Improve up/down flag validation In SMS, Signal and WhatsApp forms, reject the form if user unchecks both "alert when check goes DOWN" and "alert when check goes UP". --- hc/front/forms.py | 9 +++++++ hc/front/tests/test_add_signal.py | 28 ++++++++++++-------- hc/front/tests/test_add_sms.py | 29 +++++++++++--------- hc/front/tests/test_add_whatsapp.py | 32 ++++++++++++----------- hc/front/tests/test_edit_signal.py | 11 -------- hc/front/tests/test_edit_whatsapp.py | 11 -------- templates/integrations/email_form.html | 2 +- templates/integrations/signal_form.html | 7 ++++- templates/integrations/sms_form.html | 3 +-- templates/integrations/whatsapp_form.html | 7 ++++- 10 files changed, 73 insertions(+), 66 deletions(-) diff --git a/hc/front/forms.py b/hc/front/forms.py index 5f6f7da8..6f0641ad 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -232,6 +232,15 @@ class PhoneUpDownForm(PhoneNumberForm): up = forms.BooleanField(required=False, initial=True) down = forms.BooleanField(required=False, initial=True) + def clean(self): + super().clean() + + down = self.cleaned_data.get("down") + up = self.cleaned_data.get("up") + + if not down and not up: + self.add_error("down", "Please select at least one.") + def get_json(self): return json.dumps( { diff --git a/hc/front/tests/test_add_signal.py b/hc/front/tests/test_add_signal.py index 7355d07d..d9b7a80f 100644 --- a/hc/front/tests/test_add_signal.py +++ b/hc/front/tests/test_add_signal.py @@ -39,17 +39,6 @@ class AddSignalTestCase(BaseTestCase): # Make sure it calls assign_all_checks self.assertEqual(c.checks.count(), 1) - def test_it_obeys_up_down_flags(self): - form = {"label": "My Phone", "phone": "+1234567890"} - - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) - self.assertRedirects(r, self.channels_url) - - c = Channel.objects.get() - self.assertFalse(c.signal_notify_down) - self.assertFalse(c.signal_notify_up) - @override_settings(SIGNAL_CLI_ENABLED=False) def test_it_handles_disabled_integration(self): self.client.login(username="alice@example.org", password="password") @@ -63,3 +52,20 @@ class AddSignalTestCase(BaseTestCase): self.client.login(username="bob@example.org", password="password") r = self.client.get(self.url) self.assertEqual(r.status_code, 403) + + def test_it_handles_down_false_up_true(self): + form = {"phone": "+1234567890", "up": True} + + self.client.login(username="alice@example.org", password="password") + self.client.post(self.url, form) + + c = Channel.objects.get() + self.assertFalse(c.signal_notify_down) + self.assertTrue(c.signal_notify_up) + + def test_it_rejects_unchecked_up_and_down(self): + form = {"phone": "+1234567890"} + + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url, form) + self.assertContains(r, "Please select at least one.") diff --git a/hc/front/tests/test_add_sms.py b/hc/front/tests/test_add_sms.py index 1433ba2c..ad489b04 100644 --- a/hc/front/tests/test_add_sms.py +++ b/hc/front/tests/test_add_sms.py @@ -44,14 +44,14 @@ class AddSmsTestCase(BaseTestCase): self.assertEqual(c.checks.count(), 1) def test_it_rejects_bad_number(self): - for v in ["not a phone number address", False, 15, "+123456789A"]: + for v in ["not a phone number", False, 15, "+123456789A"]: form = {"phone": v} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertContains(r, "Invalid phone number format.") def test_it_trims_whitespace(self): - form = {"phone": " +1234567890 "} + form = {"phone": " +1234567890 ", "down": True} self.client.login(username="alice@example.org", password="password") self.client.post(self.url, form) @@ -74,7 +74,7 @@ class AddSmsTestCase(BaseTestCase): self.assertEqual(r.status_code, 403) def test_it_strips_invisible_formatting_characters(self): - form = {"label": "My Phone", "phone": "\u202c+1234567890\u202c"} + form = {"phone": "\u202c+1234567890\u202c", "down": True} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) @@ -84,7 +84,7 @@ class AddSmsTestCase(BaseTestCase): self.assertEqual(c.phone_number, "+1234567890") def test_it_strips_hyphens(self): - form = {"label": "My Phone", "phone": "+123-4567890"} + form = {"phone": "+123-4567890", "down": True} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) @@ -94,7 +94,7 @@ class AddSmsTestCase(BaseTestCase): self.assertEqual(c.phone_number, "+1234567890") def test_it_strips_spaces(self): - form = {"label": "My Phone", "phone": "+123 45 678 90"} + form = {"phone": "+123 45 678 90", "down": True} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) @@ -103,16 +103,19 @@ class AddSmsTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.phone_number, "+1234567890") - def test_it_obeys_up_down_flags(self): - form = {"label": "My Phone", "phone": "+1234567890"} + def test_it_handles_down_false_up_true(self): + form = {"phone": "+1234567890", "up": True} self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) - self.assertRedirects(r, self.channels_url) + self.client.post(self.url, form) c = Channel.objects.get() - self.assertEqual(c.kind, "sms") - self.assertEqual(c.phone_number, "+1234567890") - self.assertEqual(c.name, "My Phone") self.assertFalse(c.sms_notify_down) - self.assertFalse(c.sms_notify_up) + self.assertTrue(c.sms_notify_up) + + def test_it_rejects_unchecked_up_and_down(self): + form = {"phone": "+1234567890"} + + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url, form) + self.assertContains(r, "Please select at least one.") diff --git a/hc/front/tests/test_add_whatsapp.py b/hc/front/tests/test_add_whatsapp.py index 3ef923ab..0aeea1d7 100644 --- a/hc/front/tests/test_add_whatsapp.py +++ b/hc/front/tests/test_add_whatsapp.py @@ -55,21 +55,6 @@ class AddWhatsAppTestCase(BaseTestCase): # Make sure it calls assign_all_checks self.assertEqual(c.checks.count(), 1) - def test_it_obeys_up_down_flags(self): - form = {"label": "My Phone", "phone": "+1234567890"} - - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) - self.assertRedirects(r, self.channels_url) - - c = Channel.objects.get() - self.assertEqual(c.kind, "whatsapp") - self.assertEqual(c.phone_number, "+1234567890") - self.assertEqual(c.name, "My Phone") - self.assertFalse(c.whatsapp_notify_down) - self.assertFalse(c.whatsapp_notify_up) - self.assertEqual(c.project, self.project) - @override_settings(TWILIO_USE_WHATSAPP=False) def test_it_obeys_use_whatsapp_flag(self): self.client.login(username="alice@example.org", password="password") @@ -83,3 +68,20 @@ class AddWhatsAppTestCase(BaseTestCase): self.client.login(username="bob@example.org", password="password") r = self.client.get(self.url) self.assertEqual(r.status_code, 403) + + def test_it_handles_down_false_up_true(self): + form = {"phone": "+1234567890", "up": True} + + self.client.login(username="alice@example.org", password="password") + self.client.post(self.url, form) + + c = Channel.objects.get() + self.assertFalse(c.whatsapp_notify_down) + self.assertTrue(c.whatsapp_notify_up) + + def test_it_rejects_unchecked_up_and_down(self): + form = {"phone": "+1234567890"} + + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url, form) + self.assertContains(r, "Please select at least one.") diff --git a/hc/front/tests/test_edit_signal.py b/hc/front/tests/test_edit_signal.py index bb9f57d7..875962df 100644 --- a/hc/front/tests/test_edit_signal.py +++ b/hc/front/tests/test_edit_signal.py @@ -47,17 +47,6 @@ class EditSignalTestCase(BaseTestCase): # Make sure it does not call assign_all_checks self.assertFalse(self.channel.checks.exists()) - def test_it_obeys_up_down_flags(self): - form = {"label": "My Phone", "phone": "+1234567890"} - - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) - self.assertRedirects(r, self.channels_url) - - self.channel.refresh_from_db() - self.assertFalse(self.channel.signal_notify_down) - self.assertFalse(self.channel.signal_notify_up) - @override_settings(SIGNAL_CLI_ENABLED=False) def test_it_handles_disabled_integration(self): self.client.login(username="alice@example.org", password="password") diff --git a/hc/front/tests/test_edit_whatsapp.py b/hc/front/tests/test_edit_whatsapp.py index 88024093..4dc2dd58 100644 --- a/hc/front/tests/test_edit_whatsapp.py +++ b/hc/front/tests/test_edit_whatsapp.py @@ -54,17 +54,6 @@ class EditWhatsAppTestCase(BaseTestCase): # Make sure it does not call assign_all_checks self.assertFalse(self.channel.checks.exists()) - def test_it_obeys_up_down_flags(self): - form = {"label": "My Phone", "phone": "+1234567890"} - - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) - self.assertRedirects(r, self.channels_url) - - self.channel.refresh_from_db() - self.assertFalse(self.channel.whatsapp_notify_down) - self.assertFalse(self.channel.whatsapp_notify_up) - @override_settings(TWILIO_USE_WHATSAPP=False) def test_it_obeys_use_whatsapp_flag(self): self.client.login(username="alice@example.org", password="password") diff --git a/templates/integrations/email_form.html b/templates/integrations/email_form.html index f20292d9..239830ea 100644 --- a/templates/integrations/email_form.html +++ b/templates/integrations/email_form.html @@ -48,7 +48,7 @@ Email Settings - {{ site_name }} -
+
-
+
diff --git a/templates/integrations/sms_form.html b/templates/integrations/sms_form.html index feb49060..6744ed66 100644 --- a/templates/integrations/sms_form.html +++ b/templates/integrations/sms_form.html @@ -79,7 +79,7 @@ SMS Settings - {% site_name %} {% endif %}
-
+
-
+