Add last changed API #51
Conversation
| # Convert payload to stop percent encoding the URL for requests | ||
| payload_str = "&".join("%s=%s" % (k,v) for k,v in payload.items()) | ||
|
|
||
| r = requests.get(url, timeout=self.timeout, headers=self.headers, params=payload_str, auth=(self.api_key, '')) |
There was a problem hiding this comment.
Why don't you pass params=payload instead of building the string. Compare https://requests.readthedocs.io/en/master/user/quickstart/#passing-parameters-in-urls
|
|
||
| ``` | ||
|
|
||
| Note: Last change timestamps are a newer feature in BambooHR, and they only started recording these dates from June 5th, 2011. |
There was a problem hiding this comment.
2011 is quite some time ago. I think it's fine to drop this info from the README 😉
|
Hi @lawrence-c Thanks for your contribution and sorry for responding sooo late. One little request: could you add a test for the new API call? Cheers! |
|
Hey @horida, Thanks for the review, however I don't have access to a BambooHR system anymore, so don't think I'd be able to do anymore changes to this MR. Please feel free to take over this MR however :) |
|
Tested in sandbox account. It's working fine. Thanks @lawrence-c for implementing this. |
|
@smeggingsmegger could you please merge this PR. Thanks |
| r = requests.get(url, timeout=self.timeout, headers=self.headers, params=payload_str, auth=(self.api_key, '')) | ||
| r.raise_for_status() | ||
|
|
||
| last_changed = r.json() |
There was a problem hiding this comment.
possible crash bug if r.text is not json parsable, e.g., timeout.
There was a problem hiding this comment.
Yes, but I'd open a different issue for to improve the code because this affects all API methods. @vahedq could you open an new issue with this? And if you have a fix at hand, a PR would be highly appreciated 😬
Nevertheless, it is expected that BambooHR answers with a JSON, so not critical.
Resolves #50.
Not sure if there is a better way of converting converting the request payload to stop it from percent encoding the ISO timestamp...