Bug/clean date

[edcohen08]

This pr adds a clean date method specifically for the creation of patients with their birthdate but also general use. The clean datetime does not suffice and passing a raw date does not work with the api.

Example of failure:

Expected behavior is for the date to be converted into a string and the patient successfully created

[codecov]

Codecov Report

Merging #31 (d959b5a) into develop (ed8d9e3) will decrease coverage by 0.08%.
The diff coverage is 87.50%.

❗ Current head d959b5a differs from pull request most recent head 4f4eccf. Consider uploading reports for the commit 4f4eccf to get more accurate results

@@             Coverage Diff             @@
##           develop      #31      +/-   ##
===========================================
- Coverage    93.22%   93.14%   -0.09%     
===========================================
  Files           24       24              
  Lines         1166     1181      +15     
===========================================
+ Hits          1087     1100      +13     
- Misses          79       81       +2     

Impacted Files	Coverage Δ
welkin/util.py	86.11% <77.77%> (-3.18%)	⬇️
test/test_patients.py	97.77% <100.00%> (+0.40%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[samamorgan]

I had considered doing this when I first wrote the patient portion, but I don't think this is the right thing to do. Even though a person was born on a specific day, that day changes between timezones. For instance, if a person was born on May 31st in New York, that could very well be April 1st in Australia.

Best practice here would be to always pass in a timezone-aware datetime object. The clean_datetime method actually will translate a timezone-naive datetime into UTC properly. For example:

>>> from datetime import datetime, timezone
>>> dt = datetime.now()
>>> dt
datetime.datetime(2022, 8, 9, 18, 10, 47, 877296)
>>> dt.astimezone(tz=timezone.utc)
datetime.datetime(2022, 8, 10, 1, 10, 47, 877296, tzinfo=datetime.timezone.utc)

Thoughts?

[edcohen08]

I see what you mean and largely agree but it appears Welkin doesn't even like that. Your example doesn't work to create a patient and neither does this

welkin.Patient(firstName="Devver", lastName="Id", birthDate="2020-12-12T00:00:00.000+04:00").create()

I've only seen

welkin.Patient(firstName="Devver", lastName="Id", birthDate="2020-12-12T00:00:00.000Z").create()

work. I'd gladly be wrong but I think welkin treats it as a utc standardized date.

Edit to add more info.

This is the error:

Edit 2 because I couldn't help myself.

Enjoy this bit of irony -- note the 'date' above and my date here. one is but one isn't

[samamorgan]

Ahhh ok. So Welkin is defining a "DATE" object to be a ISO-8601 UTC Zulu string with a zero timestamp. That's silly

I view this as a bug in the Welkin API. A birthdate should be a full timestamp. If a doctor were to tell a patient "Happy Birthday!" but they're a few timezones away, the patient could respond with "Oh, thanks, but that was yesterday". Or maybe it doesn't matter, since the primary use of birth date is probably identity verification.

What do you think of this approach? Keeps us using the same logic for timestamp formatting regardless of input.

def clean_date(date: date) -> str:
    dt = datetime(date.year, date.month, date.day, 0, 0, 0)

    return clean_datetime(dt)

Either way, approved, merge at your leisure.

[edcohen08]

Sweet, thank you. Opted for your method I much prefer that.