From 2a9a544ddf103dae1f047ed7ff5a48a44b927c04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Wed, 25 Aug 2021 18:04:54 +0300 Subject: [PATCH] Add ability to edit existing email integrations --- CHANGELOG.md | 1 + hc/front/forms.py | 10 ++- hc/front/tests/test_add_email.py | 2 +- hc/front/tests/test_edit_email.py | 109 ++++++++++++++++++++++++++++++ hc/front/urls.py | 3 +- hc/front/views.py | 68 ++++++++++++------- templates/front/channels.html | 4 +- 7 files changed, 167 insertions(+), 30 deletions(-) create mode 100644 hc/front/tests/test_edit_email.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cfd7f92..d1c71cf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file. ### Improvements - Add /api/v1/badges/ endpoint (#552) +- Add ability to edit existing email integrations ### Bug Fixes - Add handling for non-latin-1 characters in webhook headers diff --git a/hc/front/forms.py b/hc/front/forms.py index df3e6eb8..d434ead2 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -140,7 +140,7 @@ class AddPushoverForm(forms.Form): return "%s|%s|%s" % (key, prio, prio_up) -class AddEmailForm(forms.Form): +class EmailForm(forms.Form): error_css_class = "has-error" value = forms.EmailField(max_length=100) down = forms.BooleanField(required=False, initial=True) @@ -155,6 +155,14 @@ class AddEmailForm(forms.Form): if not down and not up: self.add_error("down", "Please select at least one.") + def to_json(self): + d = { + "value": self.cleaned_data["value"], + "up": self.cleaned_data["up"], + "down": self.cleaned_data["down"], + } + return json.dumps(d) + class AddUrlForm(forms.Form): error_css_class = "has-error" diff --git a/hc/front/tests/test_add_email.py b/hc/front/tests/test_add_email.py index 016994fe..1dc48a8c 100644 --- a/hc/front/tests/test_add_email.py +++ b/hc/front/tests/test_add_email.py @@ -106,7 +106,7 @@ class AddEmailTestCase(BaseTestCase): # Email should *not* have been sent self.assertEqual(len(mail.outbox), 0) - def test_it_rejects_unchecked_up_and_dwon(self): + def test_it_rejects_unchecked_up_and_down(self): form = {"value": "alice@example.org"} self.client.login(username="alice@example.org", password="password") diff --git a/hc/front/tests/test_edit_email.py b/hc/front/tests/test_edit_email.py new file mode 100644 index 00000000..14df713e --- /dev/null +++ b/hc/front/tests/test_edit_email.py @@ -0,0 +1,109 @@ +import json + +from django.core import mail +from django.test.utils import override_settings + +from hc.api.models import Channel +from hc.test import BaseTestCase + + +class EditEmailTestCase(BaseTestCase): + def setUp(self): + super().setUp() + + self.channel = Channel(project=self.project, kind="email") + self.channel.value = json.dumps( + {"value": "alerts@example.org", "up": True, "down": True} + ) + self.channel.email_verified = True + self.channel.save() + + self.url = "/integrations/%s/edit/" % self.channel.code + + def test_it_shows_form(self): + self.client.login(username="alice@example.org", password="password") + r = self.client.get(self.url) + self.assertContains(r, "Get an email message when check goes up or down.") + self.assertContains(r, "alerts@example.org") + + def test_it_saves_changes(self): + form = {"value": "new@example.org", "down": "true", "up": "false"} + + 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.assertEqual(self.channel.email_value, "new@example.org") + self.assertTrue(self.channel.email_notify_down) + self.assertFalse(self.channel.email_notify_up) + + # It should send a verification link + email = mail.outbox[0] + self.assertTrue(email.subject.startswith("Verify email address on")) + self.assertEqual(email.to[0], "new@example.org") + + def test_it_skips_verification_if_email_unchanged(self): + form = {"value": "alerts@example.org", "down": "false", "up": "true"} + + self.client.login(username="alice@example.org", password="password") + self.client.post(self.url, form) + + self.channel.refresh_from_db() + self.assertEqual(self.channel.email_value, "alerts@example.org") + self.assertFalse(self.channel.email_notify_down) + self.assertTrue(self.channel.email_notify_up) + self.assertTrue(self.channel.email_verified) + + # The email address did not change, so we should skip verification + self.assertEqual(len(mail.outbox), 0) + + def test_team_access_works(self): + form = {"value": "new@example.org", "down": "true", "up": "true"} + + self.client.login(username="bob@example.org", password="password") + self.client.post(self.url, form) + + self.channel.refresh_from_db() + self.assertEqual(self.channel.email_value, "new@example.org") + + @override_settings(EMAIL_USE_VERIFICATION=False) + def test_it_hides_confirmation_needed_notice(self): + self.client.login(username="alice@example.org", password="password") + r = self.client.get(self.url) + self.assertNotContains(r, "Requires confirmation") + + @override_settings(EMAIL_USE_VERIFICATION=False) + def test_it_auto_verifies_email(self): + form = {"value": "dan@example.org", "down": "true", "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.channel.refresh_from_db() + self.assertEqual(self.channel.email_value, "dan@example.org") + + # Email should *not* have been sent + self.assertEqual(len(mail.outbox), 0) + + def test_it_auto_verifies_own_email(self): + form = {"value": "alice@example.org", "down": "true", "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.channel.refresh_from_db() + self.assertEqual(self.channel.email_value, "alice@example.org") + + # Email should *not* have been sent + self.assertEqual(len(mail.outbox), 0) + + def test_it_requires_rw_access(self): + self.bobs_membership.role = "r" + self.bobs_membership.save() + + self.client.login(username="bob@example.org", password="password") + r = self.client.get(self.url) + self.assertEqual(r.status_code, 403) diff --git a/hc/front/urls.py b/hc/front/urls.py index 6216b312..b1938d94 100644 --- a/hc/front/urls.py +++ b/hc/front/urls.py @@ -38,6 +38,7 @@ channel_urls = [ path("add_trello/settings/", views.trello_settings, name="hc-trello-settings"), path("/checks/", views.channel_checks, name="hc-channel-checks"), path("/name/", views.update_channel_name, name="hc-channel-name"), + path("/edit/", views.edit_channel, name="hc-edit-channel"), path("/edit_webhook/", views.edit_webhook, name="hc-edit-webhook"), path("/test/", views.send_test_notification, name="hc-channel-test"), path("/remove/", views.remove_channel, name="hc-remove-channel"), @@ -55,7 +56,7 @@ project_urls = [ path("add_apprise/", views.add_apprise, name="hc-add-apprise"), path("add_call/", views.add_call, name="hc-add-call"), path("add_discord/", views.add_discord, name="hc-add-discord"), - path("add_email/", views.add_email, name="hc-add-email"), + path("add_email/", views.email_form, name="hc-add-email"), path("add_linenotify/", views.add_linenotify, name="hc-add-linenotify"), path("add_matrix/", views.add_matrix, name="hc-add-matrix"), path("add_mattermost/", views.add_mattermost, name="hc-add-mattermost"), diff --git a/hc/front/views.py b/hc/front/views.py index 5d376a58..1d522543 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -951,50 +951,66 @@ def remove_channel(request, code): @login_required -def add_email(request, code): - project = _get_rw_project_for_user(request, code) +def email_form(request, code=None, channel=None): + """ Add email integration or edit an existing email integration. """ + + is_new = channel is None + if is_new: + project = _get_rw_project_for_user(request, code) + channel = Channel(project=project, kind="email") if request.method == "POST": - form = forms.AddEmailForm(request.POST) + form = forms.EmailForm(request.POST) if form.is_valid(): - channel = Channel(project=project, kind="email") - channel.value = json.dumps( - { - "value": form.cleaned_data["value"], - "up": form.cleaned_data["up"], - "down": form.cleaned_data["down"], - } - ) + if form.cleaned_data["value"] != channel.email_value: + if not settings.EMAIL_USE_VERIFICATION: + # In self-hosted setting, administator can set + # EMAIL_USE_VERIFICATION=False to disable email verification + channel.email_verified = True + elif form.cleaned_data["value"] == request.user.email: + # If the user is adding *their own* address + # we skip the verification step + channel.email_verified = True + else: + channel.email_verified = False + + channel.value = form.to_json() channel.save() - channel.assign_all_checks() - - is_own_email = form.cleaned_data["value"] == request.user.email - if is_own_email or not settings.EMAIL_USE_VERIFICATION: - # If user is subscribing *their own* address - # we can skip the verification step. - - # Additionally, in self-hosted setting, administator has the - # option to disable the email verification step altogether. + if is_new: + channel.assign_all_checks() - channel.email_verified = True - channel.save() - else: + if not channel.email_verified: channel.send_verify_link() - return redirect("hc-channels", project.code) + return redirect("hc-channels", channel.project.code) else: - form = forms.AddEmailForm() + form = forms.EmailForm( + { + "value": channel.email_value, + "up": channel.email_notify_up, + "down": channel.email_notify_down, + } + ) ctx = { "page": "channels", - "project": project, + "project": channel.project, "use_verification": settings.EMAIL_USE_VERIFICATION, "form": form, } return render(request, "integrations/add_email.html", ctx) +@login_required +def edit_channel(request, code): + channel = _get_rw_channel_for_user(request, code) + if channel.kind == "email": + return email_form(request, channel=channel) + + return HttpResponseBadRequest() + + @require_setting("WEBHOOKS_ENABLED") @login_required def add_webhook(request, code): diff --git a/templates/front/channels.html b/templates/front/channels.html index 305dfe8e..df6e1bca 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -147,6 +147,9 @@ {% if ch.kind == "webhook" and rw %} Edit {% endif %} + {% if ch.kind == "email" and rw %} + Edit + {% endif %}
{% csrf_token %}