add IsNullOrWhiteSpace guard for userId param of GetHeartRateTimeSeri…#235
Conversation
…es() This prevents errors in HttpClient if the URL is malformed by omission of userId resulting in user//activities. In this case, the current version of HttpClient exhibits the behvior of switching from https to http (causing a fitbit error)
WestDiscGolf
left a comment
WestDiscGolf
left a comment
There was a problem hiding this comment.
The other methods use the helper, as mentioned in the comment. This should not then require another isnull check on the userid :-)
| userId = "-"; | ||
| } | ||
|
|
||
| string path = $"1.1/user/{userId}/activities/heart/date/{date:yyyy-MM-dd}/{dateRangePeriod.GetStringValue()}.json"; |
There was a problem hiding this comment.
This change looks like it was needed because the path isn't being built up using the FitbitClientHelperExtensions.ToFullUrl which does this check for the userId. Example below from GetActivitiesStatsAsync ...
public async Task<ActivitiesStats> GetActivitiesStatsAsync(string encodedUserId = null)
{
string apiCall = FitbitClientHelperExtensions.ToFullUrl("/1/user/{0}/activities.json", encodedUserId);
There was a problem hiding this comment.
Makes sense to me Adam! Dwayne and I actually had a brief conversation along the lines of "there really should be a helper..." but this was a quick in & out job for me yesterday so I missed it. I'll fix the PR today to leverage the helper instead, thanks!
There was a problem hiding this comment.
Better documentation may help contributors to understand what needs to be passed in.
/// Converts the REST api resource into the fully qualified url
/// </summary>
/// <param name="apiCall">
///"Format String" with one "Format Item" as a placeholder for "UserId"
///</param>
/// <param name="encodedUserId"></param>
/// <param name="args"></param>
/// <returns>Fully qualified url</returns>```
…Extensions.ToFullUrl(). Add better documentation on ToFullUrl()
4 participants
…es()
This prevents errors in HttpClient if the URL is malformed by omission of userId resulting in user//activities. In this case, the current version of HttpClient exhibits the behvior of switching from https to http (causing a fitbit error)
fixes #235