[1.0.1] Fix single entity request after API changes

santiher · Oct 14, 2019 · eaa0b0c · eaa0b0c
1 parent f5e1de2
commit eaa0b0c
Show file tree

Hide file tree

Showing 6 changed files with 128 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 All notable changes to this project will be documented here.
 
+## [1.0.1] - 2019-10-14
+### Fixed
+- Correctly return entities when a single one is requested using a resource id
+  reflecting changes in HelpScout's API responses.
+
 ## [1.0.0] - 2019-09-18
 ### Changed
 - HelpScoutEndpointRequester now returns subentities for methods named other

diff --git a/helpscout/__init__.py b/helpscout/__init__.py
@@ -1,4 +1,4 @@
 from helpscout.client import HelpScout  # noqa
 
 
-__version__ = '1.0.0'
+__version__ = '1.0.1'
diff --git a/helpscout/client.py b/helpscout/client.py
@@ -17,6 +17,7 @@
 
 logger = logging.getLogger('HelpScout')
 EmbeddedKey = '_embedded'
+PageKey = 'page'
 
 
 class HelpScout:
@@ -169,9 +170,11 @@ def hit_(self, endpoint, method, resource_id=None, data=None, params=None):
                 params = '&'.join('%s=%s' % (k, v) for k, v in params.items())
             url = '%s?%s' % (url, params)
         headers = self._authentication_headers()
+        logger.debug('Request: %s %s' % (method, url))
         r = getattr(requests, method)(url, headers=headers, json=data)
-        logger.debug('%s %s' % (method, url))
         ok, status_code = r.ok, r.status_code
+        logger.debug(
+            'Received: %s %s (%s - %s)' % (method, url, ok, status_code))
         if status_code in (201, 204):
             yield
         elif ok:
@@ -204,7 +207,7 @@ def _results_with_pagination(self, response, method):
         dict
             The dictionary response from help scout.
         """
-        if EmbeddedKey not in response:
+        if EmbeddedKey not in response or PageKey not in response:
             yield response
             return
         if isinstance(response[EmbeddedKey], list):

diff --git a/helpscout/model.py b/helpscout/model.py
@@ -61,6 +61,12 @@ def cls(cls, entity_name, key):
         -------
         type: The object's class
         """
+        if '/' in entity_name:
+            parts = entity_name.rsplit('/')
+            entity_name = parts[-2] if len(parts) % 2 == 0 else parts[-1]
+        if '/' in key:
+            parts = key.rsplit('/')
+            key = parts[-2] if len(parts) % 2 == 0 else parts[-1]
         plural_letters = (-2 if entity_name.endswith('es') else
                           -1 if entity_name.endswith('s') else
                           None)

diff --git a/tests/helpscout/test_client.py b/tests/helpscout/test_client.py
@@ -89,7 +89,7 @@ def test_hit_no_access_token_ok(self):
         hs_path = 'helpscout.client.HelpScout.'
         hs = self._get_client()
         with patch('helpscout.client.requests') as requests, \
-                patch('helpscout.client.logger') as logger, \
+                patch('helpscout.client.logger'), \
                 patch(hs_path + '_authenticate') as auth, \
                 patch(hs_path + '_authentication_headers') as auth_headers, \
                 patch(hs_path + '_results_with_pagination') as pages:
@@ -102,7 +102,6 @@ def test_hit_no_access_token_ok(self):
             # Asserts
             auth.assert_called_once()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
             requests.get.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_called_once()
@@ -123,11 +122,18 @@ def test_hit_ok(self):
             response = requests.get.return_value = MagicMock()
             response.ok = True
             response.json.return_value = json_response = {'a': 'b'}
+            response.status_code = 200
             list(hs.hit_(endpoint, method))
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 200)'),
+                 ]
+                )
             requests.get.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_called_once()
@@ -148,15 +154,23 @@ def test_hit_resource_id_ok(self):
             response = requests.get.return_value = MagicMock()
             response.ok = True
             response.json.return_value = json_response = {'a': 'b'}
-            list(hs.hit_(endpoint, method, resource_id))
+            response.status_code = 200
+            ret = list(hs.hit_(endpoint, method, resource_id))
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 200)'),
+                 ]
+                )
             requests.get.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_called_once()
-            pages.assert_called_once_with(json_response, method)
+            pages.assert_not_called()
+            self.assertEqual(ret, [json_response])
 
     def test_hit_params_dict_ok(self):
         params, params_str = {'embed': 'threads'}, '?embed=threads'
@@ -174,11 +188,18 @@ def test_hit_params_dict_ok(self):
             response = requests.get.return_value = MagicMock()
             response.ok = True
             response.json.return_value = json_response = {'a': 'b'}
+            response.status_code = 200
             list(hs.hit_(endpoint, method, None, params=params))
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 200)'),
+                 ]
+                )
             requests.get.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_called_once()
@@ -200,15 +221,23 @@ def test_hit_resource_id_with_params_dict_ok(self):
             response = requests.get.return_value = MagicMock()
             response.ok = True
             response.json.return_value = json_response = {'a': 'b'}
-            list(hs.hit_(endpoint, method, resource_id, params=params))
+            response.status_code = 200
+            ret = list(hs.hit_(endpoint, method, resource_id, params=params))
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 200)'),
+                 ]
+                )
             requests.get.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_called_once()
-            pages.assert_called_once_with(json_response, method)
+            pages.assert_not_called()
+            self.assertEqual(ret, [json_response])
 
     def test_hit_resource_id_with_params_str_ok(self):
         params_str = 'embed=threads'
@@ -227,15 +256,24 @@ def test_hit_resource_id_with_params_str_ok(self):
             response = requests.get.return_value = MagicMock()
             response.ok = True
             response.json.return_value = json_response = {'a': 'b'}
-            list(hs.hit_(endpoint, method, resource_id, params=params_str))
+            response.status_code = 200
+            ret = list(
+                hs.hit_(endpoint, method, resource_id, params=params_str))
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 200)'),
+                 ]
+                )
             requests.get.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_called_once()
-            pages.assert_called_once_with(json_response, method)
+            pages.assert_not_called()
+            self.assertEqual(ret, [json_response])
 
     def test_hit_post_ok(self):
         endpoint, method = 'users', 'post'
@@ -257,7 +295,13 @@ def test_hit_post_ok(self):
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 201)'),
+                 ]
+                )
             requests.post.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_not_called()
@@ -284,7 +328,13 @@ def test_hit_delete_ok(self):
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 204)'),
+                 ]
+                )
             requests.delete.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_not_called()
@@ -311,7 +361,13 @@ def test_hit_patch_ok(self):
             # Asserts
             auth.assert_not_called()
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 204)'),
+                 ]
+                )
             requests.patch.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_not_called()
@@ -337,9 +393,15 @@ def test_hit_token_expired(self):
             list(hs.hit_(endpoint, method))
             # Asserts
             self.assertEqual(auth_headers.call_count, 2)
+            log_msg_body = method + ' ' + full_url
             self.assertEqual(
                 logger.debug.call_args_list,
-                [call(method + ' ' + full_url) for _ in range(2)])
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (False - 401)'),
+                 call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 200)'),
+                 ]
+                )
             self.assertEqual(
                 requests.get.call_args_list,
                 [call(full_url, headers=headers, json=None) for _ in range(2)])
@@ -367,9 +429,15 @@ def test_hit_rate_limit_exceeded(self):
             list(hs.hit_(endpoint, method))
             # Asserts
             self.assertEqual(auth_headers.call_count, 2)
+            log_msg_body = method + ' ' + full_url
             self.assertEqual(
                 logger.debug.call_args_list,
-                [call(method + ' ' + full_url) for _ in range(2)])
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (False - 429)'),
+                 call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (True - 200)'),
+                 ]
+                )
             self.assertEqual(
                 requests.get.call_args_list,
                 [call(full_url, headers=headers, json=None) for _ in range(2)])
@@ -401,7 +469,13 @@ def test_hit_exception(self):
                 list(hs.hit_(endpoint, method))
             # Asserts
             auth_headers.assert_called_once()
-            logger.debug.assert_called_once_with(method + ' ' + full_url)
+            log_msg_body = method + ' ' + full_url
+            self.assertEqual(
+                logger.debug.call_args_list,
+                [call('Request: ' + log_msg_body),
+                 call('Received: ' + log_msg_body + ' (False - 500)'),
+                 ]
+                )
             requests.get.assert_called_once_with(
                 full_url, headers=headers, json=None)
             response.json.assert_not_called()

diff --git a/tests/helpscout/test_model.py b/tests/helpscout/test_model.py
@@ -56,6 +56,24 @@ def test_class_superclass(self):
         cls = HelpScoutObject.cls('users', 'users')
         self.assertTrue(issubclass(cls, HelpScoutObject))
 
+    def test_cls_name_general_endpoint_top_level(self):
+        cls = HelpScoutObject.cls('users', 'users')
+        self.assertTrue(cls.__name__, 'users')
+
+    def test_cls_name_top_level_resource_id(self):
+        cls = HelpScoutObject.cls('users/120', 'users/120')
+        self.assertTrue(cls.__name__, 'users')
+
+    def test_cls_name_down_level_general_endpoint(self):
+        endpoint = 'conversations/120/threads'
+        cls = HelpScoutObject.cls(endpoint, endpoint)
+        self.assertTrue(cls.__name__, 'threads')
+
+    def test_cls_name_down_level_resource_id(self):
+        endpoint = 'conversations/120/threads/4'
+        cls = HelpScoutObject.cls(endpoint, endpoint)
+        self.assertTrue(cls.__name__, 'threads')
+
     def test_str(self):
         cls = HelpScoutObject.cls('users', 'users')
         user = cls({'id': 12, 'name': 'Mike'})