From ae976a38b6ac255850c1bc273360162edc2bc595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Thu, 28 Jan 2021 12:57:08 +0200 Subject: [PATCH] Fix a crash when adding an integration for an empty Trello account --- CHANGELOG.md | 1 + hc/front/forms.py | 10 ++++++++ hc/front/tests/test_add_trello.py | 27 ++++++++++++++------- hc/front/tests/test_trello_settings.py | 9 +++++++ hc/front/views.py | 13 +++++++--- static/js/add_trello.js | 9 +++---- templates/integrations/trello_settings.html | 18 +++++++++++--- 7 files changed, 65 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2cd92b..cacec69b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file. ## Bug Fixes - Fix unwanted HTML escaping in SMS and WhatsApp notifications +- Fix a crash when adding an integration for an empty Trello account ## v1.18.0 - 2020-12-09 diff --git a/hc/front/forms.py b/hc/front/forms.py index 8babe1d0..818a90d8 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -280,3 +280,13 @@ class AddZulipForm(forms.Form): def get_value(self): return json.dumps(dict(self.cleaned_data), sort_keys=True) + + +class AddTrelloForm(forms.Form): + token = forms.RegexField(regex=r"^[0-9a-fA-F]{64}$") + board_name = forms.CharField(max_length=100) + list_name = forms.CharField(max_length=100) + list_id = forms.RegexField(regex=r"^[0-9a-fA-F]{16,32}$") + + def get_value(self): + return json.dumps(dict(self.cleaned_data), sort_keys=True) diff --git a/hc/front/tests/test_add_trello.py b/hc/front/tests/test_add_trello.py index cb2c5ead..db6b13c8 100644 --- a/hc/front/tests/test_add_trello.py +++ b/hc/front/tests/test_add_trello.py @@ -18,14 +18,10 @@ class AddTrelloTestCase(BaseTestCase): def test_it_works(self): form = { - "settings": json.dumps( - { - "token": "fake-token", - "board_name": "My Board", - "list_name": "My List", - "list_id": "fake-list-id", - } - ) + "token": "0" * 64, + "board_name": "My Board", + "list_name": "My List", + "list_id": "1" * 32, } self.client.login(username="alice@example.org", password="password") @@ -34,7 +30,7 @@ class AddTrelloTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.kind, "trello") - self.assertEqual(c.trello_token, "fake-token") + self.assertEqual(c.trello_token, "0" * 64) self.assertEqual(c.project, self.project) @override_settings(TRELLO_APP_KEY=None) @@ -50,3 +46,16 @@ class AddTrelloTestCase(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_requires_board_name(self): + self.client.login(username="alice@example.org", password="password") + + form = { + "token": "0" * 64, + "board_name": "", + "list_name": "My List", + "list_id": "1" * 32, + } + + r = self.client.post(self.url, form) + self.assertEqual(r.status_code, 400) diff --git a/hc/front/tests/test_trello_settings.py b/hc/front/tests/test_trello_settings.py index 3e70725b..4d6d0bfa 100644 --- a/hc/front/tests/test_trello_settings.py +++ b/hc/front/tests/test_trello_settings.py @@ -24,3 +24,12 @@ class AddTrelloTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.get(self.url) self.assertEqual(r.status_code, 404) + + @patch("hc.front.views.requests.get") + def test_it_handles_no_lists(self, mock_get): + mock_get.return_value.json.return_value = [] + + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url) + self.assertNotContains(r, "Please select the Trello list") + self.assertContains(r, "Could not find any boards with lists") diff --git a/hc/front/views.py b/hc/front/views.py index 0b614b63..f9cd2857 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -1661,8 +1661,12 @@ def add_signal(request, code): def add_trello(request, code): project = _get_rw_project_for_user(request, code) if request.method == "POST": + form = forms.AddTrelloForm(request.POST) + if not form.is_valid(): + return HttpResponseBadRequest() + channel = Channel(project=project, kind="trello") - channel.value = request.POST["settings"] + channel.value = form.get_value() channel.save() channel.assign_all_checks() @@ -1753,14 +1757,17 @@ def trello_settings(request): { "key": settings.TRELLO_APP_KEY, "token": token, + "filter": "open", "fields": "id,name", "lists": "open", "list_fields": "id,name", } ) - r = requests.get(url) - ctx = {"token": token, "data": r.json()} + boards = requests.get(url).json() + num_lists = sum(len(board["lists"]) for board in boards) + + ctx = {"token": token, "boards": boards, "num_lists": num_lists} return render(request, "integrations/trello_settings.html", ctx) diff --git a/static/js/add_trello.js b/static/js/add_trello.js index 0804b81e..965990da 100644 --- a/static/js/add_trello.js +++ b/static/js/add_trello.js @@ -1,12 +1,9 @@ $(function() { function updateSettings() { var opt = $('#list-selector').find(":selected"); - $("#settings").val(JSON.stringify({ - "token": $("#settings").data("token"), - "list_id": opt.data("listId"), - "board_name": opt.data("boardName"), - "list_name": opt.data("listName") - })); + $("#board-name").val(opt.data("boardName")); + $("#list-name").val(opt.data("listName")); + $("#list-id").val(opt.data("listId")); } var tokenMatch = window.location.hash.match(/token=(\w+)/); diff --git a/templates/integrations/trello_settings.html b/templates/integrations/trello_settings.html index dccef416..1ea74e53 100644 --- a/templates/integrations/trello_settings.html +++ b/templates/integrations/trello_settings.html @@ -1,15 +1,19 @@ -

Authentication successful!

+{% if num_lists %} +

Authentication successful!

Please select the Trello list to post notifications to:

{% csrf_token %} - + + + +