Issue228 cleanup activity changes by dwaynefitabase · Pull Request #229 · aarondcoleman/Fitbit.NET

dwaynefitabase · 2017-09-13T23:33:51Z

FIxes #228

An extension of the changes from Redmorello

….NET-1 into redmorello-async-portable Init Merge

…yChanges

… shout be a "-" and not null

… model

…of the null values

…wanted change. Reverted back

…the heart acvtivity models for ease of use. Also decided to kill the new heart rate methods (one's that allowed for a date string or a timespan)

…aarondcoleman/Fitbit.NET into issue228-CleanupActivityChanges Took all local changes in merge

WestDiscGolf

Looks good. I think there may have been a couple of items missed from the original pull request which need tweaking.

WestDiscGolf · 2017-09-14T08:22:25Z

        public Fixture fixture { get; set; }

-        [TestFixtureSetUp]
+        [OneTimeSetUp]


New one on me :)

Our CI build agents didn't like TestFixtureSetUp used with V3 of the test framework ;-|

WestDiscGolf · 2017-09-14T08:22:55Z

+    <None Include="SampleData\ApiSubscriptionNotification.json" />
+    <None Include="SampleData\GetActivities.json" />
+    <None Include="SampleData\GetHeartRateTimeSeries.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>


Does this need to be different from the others?

WestDiscGolf · 2017-09-14T08:23:09Z

+    </None>
+    <None Include="SampleData\GetHeartRateIntradayTimeSeries.json" />
+    <None Include="SampleData\GetActivityLogsList.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>


Same as above? need to be different?

WestDiscGolf · 2017-09-14T08:24:18Z

        Task<ApiSubscription> AddSubscriptionAsync(APICollectionType apiCollectionType, string uniqueSubscriptionId, string subscriberId = default(string));
        Task DeleteSubscriptionAsync(APICollectionType collection, string uniqueSubscriptionId, string subscriberId = null);
+        Task<ActivityLog> LogActivityAsync(ActivityLog model);
+        Task<HeartActivitiesTimeSeries> GetHeartRateTimeSeries(DateTime date, DateRangePeriod dateRangePeriod, string userId = "-");


GetHeartRateTimeSeriesAsync to keep with the pattern

GetHeartRateTimeSeries is the name used in current versions. RedMorello updated it, but this is a braking change so we're gonna keep it the same for now.

Fitbit.NET/Fitbit.Portable/FitbitClient.cs

Line 396 in 079c870

public async Task<HeartActivitiesTimeSeries> GetHeartRateTimeSeries(DateTime date, DateRangePeriod dateRangePeriod, string userId = null)

WestDiscGolf · 2017-09-14T08:24:31Z

        Task DeleteSubscriptionAsync(APICollectionType collection, string uniqueSubscriptionId, string subscriberId = null);
+        Task<ActivityLog> LogActivityAsync(ActivityLog model);
+        Task<HeartActivitiesTimeSeries> GetHeartRateTimeSeries(DateTime date, DateRangePeriod dateRangePeriod, string userId = "-");
+        Task<HeartActivitiesIntraday> GetHeartRateIntraday(DateTime date, HeartRateResolution resolution, string userId = "-");


GetHeartRateIntradayAsync to keep with the pattern

Fitbit.NET/Fitbit.Portable/FitbitClient.cs

Line 422 in 079c870

public async Task<HeartActivitiesIntraday> GetHeartRateIntraday(DateTime date, HeartRateResolution resolution)

WestDiscGolf · 2017-09-14T08:25:45Z

+        public int Value { get; set; }
    }

+    public class IntradayActivitiesHeart


All classes should be in their own files :)

WestDiscGolf · 2017-09-14T08:26:12Z

+        [JsonProperty(PropertyName = "activities-heart-intraday")]
+        public ActivitesHeartIntraday ActivitiesHeartIntraday { get; set; }
+    }
+    public class ActivitesHeart


All classes should be in their own files

WestDiscGolf · 2017-09-14T08:28:29Z

+        public async Task<List<ActivityLogsList>> GetActivityLogsList(DateTime? beforeDate, DateTime? afterDate, int limit = 20, string encodedUserId = default(string))
+        {
+            var apiCall = string.Empty;
+            limit = limit > 20 ? 20 : limit;


Missing the check for negative values as per the original code review / pull request

WestDiscGolf · 2017-09-14T08:28:50Z

+            return (new JsonDotNetSerializer() { RootProperty = "activities" }).Deserialize<List<ActivityLogsList>>(responseBody);
+        }
+
+


Extra whitespace not needed between methods

WestDiscGolf · 2017-09-14T08:29:05Z

+        /// <returns></returns>
+        public async Task<HeartActivitiesTimeSeries> GetHeartRateTimeSeries(DateTime date, DateRangePeriod dateRangePeriod, string userId = "-")
+        {
+


Extra whitespace not needed in methods

aarondcoleman

looks good!

redmorello and others added 30 commits March 10, 2017 12:35

Added new LogActivityAsync, GetActivityLogsList, GetHeartRate methods

15792d2

Removed url string

c090a6c

Update to Newtownsoft

2be4b3d

Start of adding some unit tests for new methods

5fcce94

Added unit tests for new methods

c0d332a

Merge branch 'async-portable' of https://github.com/redmorello/Fitbit…

733823d

….NET-1 into redmorello-async-portable Init Merge

Merge branch 'redmorello-async-portable' into issue228-CleanupActivit…

ea82962

…yChanges

According to the API docs. The deault to represent the loggin in user…

b347878

… shout be a "-" and not null

Renamed Value to HeartActivitesValue

6588632

Updated type of CustomHeartRateZones

2b04c78

Updated GetHeartActivitiesTimeSeries to work with new HeartActivities…

30d5060

… model

revert encodedUserId to default(string) since "ToFullUrl" takes care …

ee9a86e

…of the null values

Deleted file because the format did not match the api

0d3d0be

New model to replace old Intraday model. This one matches the API

3f28806

Project changes for the new model

7f336a6

Code to update Heart Rate Intraday

17f1a5d

Update code for HeartRateTimeSeries

1ccc853

Updated client to reflect changes to the heart rate api structure

8024906

Removed old intraday code

42393b4

Updated activity list model to match the api docs

03aaf35

Int to long

d904a57

New functions to aloow for string dates to be passed

6814d67

Extra tag broke csproj load

917e05e

WIP: Started fixing tests for Heartrate

85bd7bc

Refactor/Rename changed Value to HeartActivitiesValue. This was an un…

d61780a

…wanted change. Reverted back

Changes date prop from string to DateTIme

fceb10a

Fixed build errors that occured from changes to heart rate models

c80a2e5

Added some instructions for use

bd08eab

Updated tests to validate HeartActivitiesIntradayTimeSeries

c2e5458

Changed Calories out to double to match API response

dc13725

dwaynelavon added 11 commits August 28, 2017 14:07

Include GetHeartRateTimeSeries1.1.json in output directory

cc915fa

Test and random changes before rollback

d1e1c5c

Partial rollback of changes, Decided to keep the previous version of …

1d6e23e

…the heart acvtivity models for ease of use. Also decided to kill the new heart rate methods (one's that allowed for a date string or a timespan)

Merge branch 'issue228-CleanupActivityChanges' of https://github.com/…

5596520

…aarondcoleman/Fitbit.NET into issue228-CleanupActivityChanges Took all local changes in merge

Updated Activity tests

896ead1

Removed dup System.Threading.Tasks import in CSPROJ

36bcf3b

Added System.Threading.Tasks back into CSPROJ

5c70c79

Removed f as calories is now represented as a double

4c3143f

Removed obsolete attributes

112ab43

Update NUnit

b6126e6

Updated props to more closely resemble what Fitabase needs

4aa604d

dwaynefitabase requested a review from aarondcoleman September 13, 2017 23:33

WestDiscGolf reviewed Sep 14, 2017

View reviewed changes

dwaynelavon added 7 commits September 14, 2017 09:55

One class per file

067c428

Naming conventions -- include Async

52113ad

More formatting

505eb8b

Negative number check and formatting

71c02c5

comments and formatting

7901fff

Unused class

e1177b7

Remove old commented code

2d58204

aarondcoleman approved these changes Sep 14, 2017

View reviewed changes

dwaynelavon added 2 commits September 14, 2017 10:35

One class per file

42c34db

Imports

37b9eaf

aarondcoleman merged commit 0fdb2a0 into master Sep 14, 2017

aarondcoleman deleted the issue228-CleanupActivityChanges branch September 14, 2017 18:28

aarondcoleman mentioned this pull request Jan 6, 2019

Update, Tidy and Tweak #246

Open

		return (new JsonDotNetSerializer() { RootProperty = "activities" }).Deserialize<List<ActivityLogsList>>(responseBody);
		}

Conversation

dwaynefitabase commented Sep 13, 2017

Uh oh!

WestDiscGolf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aarondcoleman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants