fix(auth): Add lastRefreshTime to UserMetadata toJSON method. #889
Conversation
It's returned from the server in a different format than the other timestamps. Fixes #887
|
Note the diffbase. (Will retarget to master after that other one lands) |
go/small-cls? But more relevantly, my fix for the importing one isn't working and will take a bit longer than these two more trivial ones. |
…ta_tojson_lastrefreshtime
| }); | ||
| const expectedMetadataJSON = { | ||
| lastSignInTime: new Date(expectedLastLoginAt).toUTCString(), | ||
| creationTime: new Date(expectedCreatedAt).toUTCString(), | ||
| lastRefreshTime: new Date(expectedLastRefreshAt).toUTCString(), |
There was a problem hiding this comment.
We are lacking test cases for lastRefreshTime. Most existing test cases only check for the other 2 fields. We might want to address that.
There was a problem hiding this comment.
Looking at the code now (2y later) I see some test cases for this, but perhaps not as many as there should be. I think that's independent of this PR though.
| @@ -80,6 +80,7 @@ function getValidUserResponse(tenantId?: string): GetAccountInfoUserResponse { | |||
| validSince: '1476136676', | |||
| lastLoginAt: '1476235905000', | |||
| createdAt: '1476136676000', | |||
| lastRefreshAt: '2016-10-12T01:31:45.000Z', | |||
There was a problem hiding this comment.
Unrelated to this PR. But are the fields like createdAt actually sent as strings (quoted), not numbers?
There was a problem hiding this comment.
Yeah they are all sent as strings. Here is a snippet:
"lastLoginAt": "1590622314664",
"createdAt": "1590545248026",
"lastRefreshAt": "2020-05-28T01:44:39.262Z"
|
@rsgowman do you know the status of this fix? I am using the RTDB Admin SDK and am still not receiving the lastRefreshTime. |
…ta_tojson_lastrefreshtime
|
@cainenielsen sorry, looks like this stalled out. Let me see if I can update it and find the right person to approve it. |
Since specifying this when importing users isn't supported.
rsgowman
force-pushed
the
rsgowman/usermetadata_tojson_lastrefreshtime
branch
from
02031d1
to
f072d6b
Compare
|
@lahirumaramba, could you review this old PR (or redirect appropriately)? Thanks! |
|
@rsgowman Thank you for surfacing this. LGTM. I will merge this. |
lahirumaramba
changed the title
See #887