Skip to content

Commit

Permalink
Mailroom client should use content-type header on responses to know w…
Browse files Browse the repository at this point in the history 
…hether to parse as JSON
  • Loading branch information
@rowanseymour
rowanseymour committed May 23, 2024
1 parent 1ddfdad commit c427867
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 166 deletions.
58 changes: 27 additions & 31 deletions temba/channels/types/plivo/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
from django.urls import reverse

from temba.channels.models import Channel
from temba.tests import MockResponse, TembaTest
from temba.utils import json
from temba.tests import MockJsonResponse, MockResponse, TembaTest

from .type import PlivoType

Expand All @@ -25,7 +24,7 @@ def test_claim(self):
self.assertContains(response, claim_plivo_url)

with patch("requests.get") as plivo_get:
plivo_get.return_value = MockResponse(400, {})
plivo_get.return_value = MockJsonResponse(400, {})
response = self.client.get(claim_plivo_url)

self.assertEqual(response.status_code, 302)
Expand All @@ -35,7 +34,7 @@ def test_claim(self):
self.assertEqual(response.request["PATH_INFO"], connect_plivo_url)

with patch("requests.get") as plivo_get:
plivo_get.return_value = MockResponse(400, json.dumps(dict()))
plivo_get.return_value = MockJsonResponse(400, {})

# try hit the claim page, should be redirected; no credentials in session
response = self.client.get(claim_plivo_url, follow=True)
Expand All @@ -45,18 +44,16 @@ def test_claim(self):
# let's add a number already connected to the account
with patch("requests.get") as plivo_get:
with patch("requests.post") as plivo_post:
plivo_get.return_value = MockResponse(
plivo_get.return_value = MockJsonResponse(
200,
json.dumps(
dict(
objects=[
dict(number="16062681435", region="California, UNITED STATES"),
dict(number="8080", region="GUADALAJARA, MEXICO"),
]
)
dict(
objects=[
dict(number="16062681435", region="California, UNITED STATES"),
dict(number="8080", region="GUADALAJARA, MEXICO"),
]
),
)
plivo_post.return_value = MockResponse(202, json.dumps(dict(status="changed", app_id="app-id")))
plivo_post.return_value = MockJsonResponse(202, dict(status="changed", app_id="app-id"))

# make sure our numbers appear on the claim page
response = self.client.get(claim_plivo_url)
Expand Down Expand Up @@ -107,23 +104,22 @@ def test_claim(self):

with patch("temba.channels.views.requests.get") as mock_get:
with patch("temba.channels.views.requests.post") as mock_post:
response_body = json.dumps(
{
"status": "fulfilled",
"message": "created",
"numbers": [{"status": "Success", "number": "27816855210"}],
"api_id": "4334c747-9e83-11e5-9147-22000acb8094",
}
)
response_body = {
"status": "fulfilled",
"message": "created",
"numbers": [{"status": "Success", "number": "27816855210"}],
"api_id": "4334c747-9e83-11e5-9147-22000acb8094",
}

mock_get.side_effect = [
MockResponse(200, json.dumps(dict())), # get account
MockResponse(400, json.dumps(dict())), # failed get number
MockResponse(200, json.dumps(dict())), # successful get number after buying it
MockJsonResponse(200, {}), # get account
MockJsonResponse(400, {}), # failed get number
MockJsonResponse(200, {}), # successful get number after buying it
]
mock_post.side_effect = [
MockResponse(200, json.dumps(dict(app_id="app-id"))), # create application
MockResponse(201, json.dumps(dict())), # buy number
MockResponse(202, response_body), # update number
MockJsonResponse(200, {"app_id": "app-id"}), # create application
MockJsonResponse(201, {}), # buy number
MockJsonResponse(202, response_body), # update number
]

# claim it the US number
Expand Down Expand Up @@ -184,16 +180,16 @@ def test_search(self, mock_get):

search_url = reverse("channels.types.plivo.search")

mock_get.return_value = MockResponse(
200, json.dumps({"objects": [{"number": "16331111111"}, {"number": "16332222222"}]})
mock_get.return_value = MockJsonResponse(
200, {"objects": [{"number": "16331111111"}, {"number": "16332222222"}]}
)

response = self.client.post(search_url, {"country": "US", "pattern": ""})
self.assertEqual(response.status_code, 200)
self.assertEqual(["+1 633-111-1111", "+1 633-222-2222"], response.json())

# missing key to throw exception
mock_get.return_value = MockResponse(200, json.dumps({}))
mock_get.return_value = MockJsonResponse(200, {})
response = self.client.post(search_url, {"country": "US", "pattern": ""})

self.assertEqual(response.status_code, 200)
Expand Down Expand Up @@ -224,7 +220,7 @@ def test_connect_plivo(self):

# ok, now with a success
with patch("requests.get") as mock_get:
mock_get.return_value = MockResponse(200, json.dumps(dict()))
mock_get.return_value = MockJsonResponse(200, {})
response = self.client.post(connect_url, dict(auth_id="auth-id", auth_token="auth-token"))

# plivo should be added to the session
Expand Down
4 changes: 2 additions & 2 deletions temba/channels/types/rocketchat/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from django.urls import reverse

from temba.channels.models import Channel
from temba.tests import MockResponse, TembaTest
from temba.tests import MockJsonResponse, MockResponse, TembaTest

from .client import Client, ClientError
from .type import RocketChatType
Expand Down Expand Up @@ -62,7 +62,7 @@ def new_channel(self, config=None) -> Channel:
class ClientTest(RocketChatMixin):
@patch("requests.put")
def test_settings_success(self, mock_request):
mock_request.return_value = MockResponse(204, {})
mock_request.return_value = MockJsonResponse(204, {})
try:
Client(self.secure_url, self.secret).settings("http://temba.io/c/1234-5678", "test-bot")
except ClientError:
Expand Down
72 changes: 33 additions & 39 deletions temba/channels/types/whatsapp/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from django.urls import reverse

from temba.request_logs.models import HTTPLog
from temba.tests import MockResponse, TembaTest
from temba.tests import MockJsonResponse, MockResponse, TembaTest
from temba.utils.views import TEMBA_MENU_SELECTION

from ...models import Channel
Expand Down Expand Up @@ -40,7 +40,7 @@ def test_claim(self, mock_randint):
self.assertNotContains(response, claim_whatsapp_cloud_url)

with patch("requests.get") as wa_cloud_get:
wa_cloud_get.return_value = MockResponse(400, {})
wa_cloud_get.return_value = MockJsonResponse(400, {})
response = self.client.get(claim_whatsapp_cloud_url)

self.assertEqual(response.status_code, 302)
Expand All @@ -51,7 +51,7 @@ def test_claim(self, mock_randint):

self.make_beta(self.admin)
with patch("requests.get") as wa_cloud_get:
wa_cloud_get.return_value = MockResponse(400, {})
wa_cloud_get.return_value = MockJsonResponse(400, {})
response = self.client.get(claim_whatsapp_cloud_url)

self.assertEqual(response.status_code, 302)
Expand All @@ -62,54 +62,48 @@ def test_claim(self, mock_randint):

with patch("requests.get") as wa_cloud_get:
wa_cloud_get.side_effect = [
MockResponse(400, {}),
MockJsonResponse(400, {}),
# missing permissions
MockResponse(
MockJsonResponse(
200,
json.dumps({"data": {"scopes": []}}),
{"data": {"scopes": []}},
),
# success
MockResponse(
MockJsonResponse(
200,
json.dumps(
{
"data": {
"scopes": [
"business_management",
"whatsapp_business_management",
"whatsapp_business_messaging",
]
}
{
"data": {
"scopes": [
"business_management",
"whatsapp_business_management",
"whatsapp_business_messaging",
]
}
),
},
),
MockResponse(
MockJsonResponse(
200,
json.dumps(
{
"data": {
"scopes": [
"business_management",
"whatsapp_business_management",
"whatsapp_business_messaging",
]
}
{
"data": {
"scopes": [
"business_management",
"whatsapp_business_management",
"whatsapp_business_messaging",
]
}
),
},
),
MockResponse(
MockJsonResponse(
200,
json.dumps(
{
"data": {
"scopes": [
"business_management",
"whatsapp_business_management",
"whatsapp_business_messaging",
]
}
{
"data": {
"scopes": [
"business_management",
"whatsapp_business_management",
"whatsapp_business_messaging",
]
}
),
},
),
]
response = self.client.get(connect_whatsapp_cloud_url)
Expand Down
12 changes: 6 additions & 6 deletions temba/flows/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from temba.orgs.integrations.dtone import DTOneType
from temba.orgs.models import Export
from temba.templates.models import Template, TemplateTranslation
from temba.tests import CRUDLTestMixin, MockResponse, TembaTest, matchers, mock_mailroom, override_brand
from temba.tests import CRUDLTestMixin, MockJsonResponse, TembaTest, matchers, mock_mailroom, override_brand
from temba.tests.base import get_contact_search
from temba.tests.engine import MockSessionWriter
from temba.tests.s3 import MockS3Client, jsonlgz_encode
Expand Down Expand Up @@ -5156,7 +5156,7 @@ def test_simulation_ivr(self):

with override_settings(MAILROOM_AUTH_TOKEN="sesame", MAILROOM_URL="https://mailroom.temba.io"):
with patch("requests.post") as mock_post:
mock_post.return_value = MockResponse(200, '{"session": {}}')
mock_post.return_value = MockJsonResponse(200, {"session": {}})
response = self.client.post(url, payload, content_type="application/json")

self.assertEqual(response.status_code, 200)
Expand Down Expand Up @@ -5194,13 +5194,13 @@ def test_simulation(self):

with override_settings(MAILROOM_AUTH_TOKEN="sesame", MAILROOM_URL="https://mailroom.temba.io"):
with patch("requests.post") as mock_post:
mock_post.return_value = MockResponse(400, '{"session": {}}')
mock_post.return_value = MockJsonResponse(400, {"session": {}})
response = self.client.post(url, json.dumps(payload), content_type="application/json")
self.assertEqual(500, response.status_code)

# start a flow
with patch("requests.post") as mock_post:
mock_post.return_value = MockResponse(200, '{"session": {}}')
mock_post.return_value = MockJsonResponse(200, {"session": {}})
response = self.client.post(url, json.dumps(payload), content_type="application/json")
self.assertEqual(200, response.status_code)
self.assertEqual({}, response.json()["session"])
Expand All @@ -5226,12 +5226,12 @@ def test_simulation(self):
}

with patch("requests.post") as mock_post:
mock_post.return_value = MockResponse(400, '{"session": {}}')
mock_post.return_value = MockJsonResponse(400, {"session": {}})
response = self.client.post(url, json.dumps(payload), content_type="application/json")
self.assertEqual(500, response.status_code)

with patch("requests.post") as mock_post:
mock_post.return_value = MockResponse(200, '{"session": {}}')
mock_post.return_value = MockJsonResponse(200, {"session": {}})
response = self.client.post(url, json.dumps(payload), content_type="application/json")
self.assertEqual(200, response.status_code)
self.assertEqual({}, response.json()["session"])
Expand Down
12 changes: 5 additions & 7 deletions temba/mailroom/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ def msg_send(self, org_id: int, user_id: int, contact_id: int, text: str, attach
def po_export(self, org_id: int, flow_ids: list, language: str):
payload = {"org_id": org_id, "flow_ids": flow_ids, "language": language}

return self._request("po/export", payload, returns_json=False)
return self._request("po/export", payload)

def po_import(self, org_id, flow_ids, language, po_data):
payload = {"org_id": org_id, "flow_ids": flow_ids, "language": language}
Expand Down Expand Up @@ -380,7 +380,7 @@ def ticket_reopen(self, org_id, user_id, ticket_ids):

return self._request("ticket/reopen", payload)

def _request(self, endpoint, payload=None, files=None, post=True, encode_json=False, returns_json=True):
def _request(self, endpoint, payload=None, files=None, post=True, encode_json=False):
if logger.isEnabledFor(logging.DEBUG): # pragma: no cover
logger.debug("=============== %s request ===============" % endpoint)
logger.debug(json.dumps(payload, indent=2))
Expand All @@ -400,12 +400,10 @@ def _request(self, endpoint, payload=None, files=None, post=True, encode_json=Fa
req_fn = requests.post if post else requests.get
response = req_fn("%s/mr/%s" % (self.base_url, endpoint), headers=headers, **kwargs)

if returns_json:
try:
resp_body = response.json()
except Exception:
resp_body = response.content
if response.headers.get("Content-Type") == "application/json":
resp_body = response.json()
else:
# not all endpoints return JSON, e.g. po file export
resp_body = response.content

if response.status_code == 422:
Expand Down
Loading

0 comments on commit c427867

Please sign in to comment.