UID2-3497: Support for /identity/buckets #46
Conversation
caroline-ttd
changed the title
tests/test_identity_map_client.py
Outdated
| self.assertTrue(len(response.buckets) == 0) | ||
| self.assertTrue(response.is_success) | ||
|
|
||
| def test_identity_buckets_invalid_timestamp(self): |
There was a problem hiding this comment.
I think this would be better as a unit test.
There was a problem hiding this comment.
I added more test cases in test_identity_buckets_invalid_timestamp
There was a problem hiding this comment.
I think we should move test_identity_buckets_invalid_timestamp and test_get_datetime_utc_iso_format_timestamp out of IdentityMapIntegrationTests and to a unit test class.
The other tests in this class make actual API calls to an operator.
test_identity_buckets_invalid_timestamp is testing parameter validation. It shouldn't require environment variables to be set in order to run it.
test_get_datetime_utc_iso_format_timestamp doesn't test get_identity_buckets, it tests the helper method. I think we should replace test_get_datetime_utc_iso_format_timestamp with a unit test for get_identity_buckets that validates the format of the timestamp sent in the request body.
| @@ -119,3 +121,9 @@ def normalize_and_hash_phone(phone): | |||
| if not is_phone_number_normalized(phone): | |||
| raise ValueError("phone number is not normalized: " + phone) | |||
| return get_base64_encoded_hash(phone) | |||
|
|
|||
|
|
|||
| def get_datetime_utc_iso_format(timestamp): | |||
There was a problem hiding this comment.
I checked how Python's @patch and call_args work together. Creating a single simple method might be easier for this unit tests.
uid2_client/input_util.py
Outdated
| @@ -1,6 +1,8 @@ | |||
| import hashlib | |||
| import base64 | |||
|
|
|||
| import pytz | |||
There was a problem hiding this comment.
Do we need this new dependency, or can we just do:
dt_utc = timestamp.astimezone(timezone.utc)
New dependencies need to be declared in pyproject.toml.
There was a problem hiding this comment.
Nope, we need to remove the timezone, or it will throw an exception HTTP Error 400: Bad Request. Thank you for pointing out the need to add the new dependencies. I had previously installed pytz on my local, which is why it worked for me.
There was a problem hiding this comment.
Right, but I'm talking about the line that converts the timestamp to UTC, not the line that removes the timezone (dt_utc_without_tz = dt_utc.replace(tzinfo=None)).
I'm talking about replacing
dt_utc = timestamp.astimezone(pytz.utc)
with
dt_utc = timestamp.astimezone(timezone.utc)
so that we don't need to add a new dependency.
tests/test_identity_map_client.py
Outdated
| self.assertTrue(len(response.buckets) == 0) | ||
| self.assertTrue(response.is_success) | ||
|
|
||
| def test_identity_buckets_invalid_timestamp(self): |
There was a problem hiding this comment.
I think we should move test_identity_buckets_invalid_timestamp and test_get_datetime_utc_iso_format_timestamp out of IdentityMapIntegrationTests and to a unit test class.
The other tests in this class make actual API calls to an operator.
test_identity_buckets_invalid_timestamp is testing parameter validation. It shouldn't require environment variables to be set in order to run it.
test_get_datetime_utc_iso_format_timestamp doesn't test get_identity_buckets, it tests the helper method. I think we should replace test_get_datetime_utc_iso_format_timestamp with a unit test for get_identity_buckets that validates the format of the timestamp sent in the request body.
No description provided.