From bff59c92ed7a53fe49444ebd13a5bf972a4c0bfd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Wed, 25 Oct 2023 14:20:51 +0300
Subject: [PATCH] Improve API error handling in hc.front.views.trello_settings
---
hc/front/tests/test_trello_settings.py | 21 ++++++++++++-----
hc/front/views.py | 25 ++++++++++++++++++---
templates/integrations/trello_settings.html | 5 +++++
3 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/hc/front/tests/test_trello_settings.py b/hc/front/tests/test_trello_settings.py
index cbff51bd..3fd5a5cf 100644
--- a/hc/front/tests/test_trello_settings.py
+++ b/hc/front/tests/test_trello_settings.py
@@ -1,10 +1,11 @@
from __future__ import annotations
+import json
from unittest.mock import Mock, patch
from django.test.utils import override_settings
-from hc.test import BaseTestCase
+from hc.test import BaseTestCase, nolog
@override_settings(TRELLO_APP_KEY="foo")
@@ -13,9 +14,9 @@ class AddTrelloTestCase(BaseTestCase):
@patch("hc.front.views.curl.get", autospec=True)
def test_it_works(self, mock_get: Mock) -> None:
- mock_get.return_value.json.return_value = [
- {"name": "My Board", "lists": [{"name": "Alerts"}]}
- ]
+ mock_get.return_value.content = json.dumps(
+ [{"id": "1", "name": "My Board", "lists": [{"id": "2", "name": "Alerts"}]}]
+ )
self.client.login(username="alice@example.org", password="password")
r = self.client.post(self.url)
@@ -30,9 +31,19 @@ class AddTrelloTestCase(BaseTestCase):
@patch("hc.front.views.curl.get", autospec=True)
def test_it_handles_no_lists(self, mock_get: Mock) -> None:
- mock_get.return_value.json.return_value = []
+ mock_get.return_value.content = "[]"
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")
+
+ @nolog
+ @patch("hc.front.views.curl.get", autospec=True)
+ def test_it_handles_unexpected_response_from_trello(self, mock_get: Mock) -> None:
+ for sample in ("surprise", "{}", """{"lists": "surprise"}"""):
+ mock_get.return_value.content = sample
+
+ self.client.login(username="alice@example.org", password="password")
+ r = self.client.post(self.url)
+ self.assertContains(r, "Received an unexpected response from Trello")
diff --git a/hc/front/views.py b/hc/front/views.py
index c2bd2534..a9ffd4d9 100644
--- a/hc/front/views.py
+++ b/hc/front/views.py
@@ -40,7 +40,7 @@ from django.urls import reverse
from django.utils.timezone import now
from django.views.decorators.csrf import csrf_exempt
from django.views.decorators.http import require_POST
-from pydantic import BaseModel, ValidationError
+from pydantic import BaseModel, TypeAdapter, ValidationError
from hc.accounts.http import AuthenticatedHttpRequest
from hc.accounts.models import Member, Profile, Project
@@ -2274,6 +2274,20 @@ def add_apprise(request: AuthenticatedHttpRequest, code: UUID) -> HttpResponse:
return render(request, "integrations/add_apprise.html", ctx)
+class TrelloList(BaseModel):
+ id: str
+ name: str
+
+
+class TrelloBoard(BaseModel):
+ id: str
+ name: str
+ lists: list[TrelloList]
+
+
+TrelloBoards = TypeAdapter(list[TrelloBoard])
+
+
@require_setting("TRELLO_APP_KEY")
@login_required
@require_POST
@@ -2291,9 +2305,14 @@ def trello_settings(request: AuthenticatedHttpRequest) -> HttpResponse:
"list_fields": "id,name",
}
- boards = curl.get(url, params).json()
- num_lists = sum(len(board["lists"]) for board in boards)
+ result = curl.get(url, params)
+ try:
+ boards = TrelloBoards.validate_json(result.content)
+ except ValidationError:
+ logger.warning("Unexpected Trello API response: %s", result.text)
+ return render(request, "integrations/trello_settings.html", {"error": 1})
+ 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/templates/integrations/trello_settings.html b/templates/integrations/trello_settings.html
index 1ea74e53..52eabb16 100644
--- a/templates/integrations/trello_settings.html
+++ b/templates/integrations/trello_settings.html
@@ -39,6 +39,11 @@
</div>
</div>
</form>
+{% elif error %}
+<p class="alert alert-warning">
+ Could not retrieve data from your Trello account.
+ Received an unexpected response from Trello.
+</p>
{% else %}
<p class="alert alert-warning">
Could not find any boards with lists in your Trello account.