Fixed invalid date formats for GetAccountActivities

[ajm113]

Currently the function is using .String() for date fields which is a invalid date format of 2006-01-02 15:04:05.999999999 -0700 MST for the API. I updated the fields to explicitly use RFC3339 which is a perfectly valid format. Tested using go1.19.1.

[gnvk]

Hey 👋 ! Thank you for your contribution.

I cannot reproduce the bug with the current implementation. The API seems to be working fine with the current string format.

However, I do think that explicit is better than implicit, and using RFC-3339 where possible makes sense to me.

Can you please extend the unit test TestGetAccountActivities with a case where you specify at least one of these date arguments and assert that the request URL looks like as you expect.

[ajm113]

Hello @gnvk! Thanks for taking the time to review my PR!

That's pretty interesting issue can't be reproduced. Do you mind sharing a code sample? Just want to make sure it's not some mistake on my part.

I was reading up a little bit of documentation and the history of our func (Time) String() string function and it hasn't changed since 6 years ago, and the current documentation recommends only using this function for debugging purposes. Also I noticed func (Time) Format(string) string was being used in other places in the library. So I thought this was maybe something overlooked when wrote originally and this function isn't used by a lot of people? Either way, I'll be happy to write some tests around this. :smile:

Just for fun this is how I was using it:

	actionType := "FILL"
	t := time.Now()
	date := time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, t.Location())

	activity, err := e.tradeClient.GetAccountActivities(&actionType, &alpaca.AccountActivitiesRequest{
		After: &date,
	})

Thanks again, and I love the library so far!

[gnvk]

the current documentation recommends only using this function for debugging purposes

Agreed, I'm happy for your change.

Either way, I'll be happy to write some tests around this. 😄

👍

	actionType := "FILL"
	t := time.Now()
	date := time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, t.Location())

	activity, err := e.tradeClient.GetAccountActivities(&actionType, &alpaca.AccountActivitiesRequest{
		After: &date,
	})

This code works on my machine. Obviously, the location depends on your location. I tried with UTC, CEST and EST, all of them worked.

Can you share with me

• date.String() before you call the function
• the error you're getting?

[ajm113]

Ah I must of ran on old build when I was trying to test with the .Date function... Works as expected now actually, but it's a little bit of a beginner trap because .Now() will include monotonic clock reading by default which is the bit the API doesn't like.

With only .Now(): 2022-10-06 08:14:39.027854802 -0700 MST m=+0.953884149 Creates this error: after is in an invalid format. Consider formatting your timestamp more explicitly, like: '2006-01-02T15:04:05Z'

With using .Date(): 2022-10-06 00:00:00 -0700 MST

I'll update the tests with my PR a little later today. Sorry for the confusion!

[ajm113]

Hi @gnvk! Sorry for the delay. Got caught up in work yesterday. Added the tests! Not sure how I feel around the response body, but let me know if that works for you! :wink:

[ajm113]

Sorry, just noticed the test borking out after my UTC update... Fixed now @gnvk

[ajm113]

No problem! I'll be putting together another PR sometime in next coming weeks to update the other endpoints to reflect the date changes to help keep things consistent unless someone else comes in and beats me to it. ;) Thank you!