Freeze bandwidth at point in time, display in correct time zone and u… #21
Conversation
There was a problem hiding this comment.
Regarding 1, as far as I understand, bandwidth is a real time event. We don't cache the value of ttlSeconds and use it later, so it won't drift over time.
Anyway, I agree that evaluating the expires time immediately is a better solution. I'm just not sure if the drift over time issue is actually happening.
| val expiresAtString: String | ||
|
|
||
| init { | ||
| val format = DateFormat.getDateTimeInstance(DateFormat.SHORT, DateFormat.SHORT); |
There was a problem hiding this comment.
I normally don't want to include UI logic in the model class, but I guess in this case it's worth it then.
|
|
||
| init { | ||
| val format = DateFormat.getDateTimeInstance(DateFormat.SHORT, DateFormat.SHORT); | ||
| // the ttlSeconds from Go is relative to the system timezone, adjust accordingly for correct |
There was a problem hiding this comment.
Hmm, not sure I fully understand this part. I thought ttlSeconds is the number of seconds remaining before the data cap is reset. If that's the case, ttlSeconds shouldn't be affected by the timezone right?
How does it work? Do we reset data for all users at the same time? Or do we reset data for each user independently depends on their timezone?
There was a problem hiding this comment.
The reset happens individually for each device (not exactly the same thing as user). Different devices will reset either daily, weekly or monthly, and they reset at midnight local time (based on the timezone that the application reported to our server).
I'm glad you asked - looks like there's actually a deeper error:
DEBUG flashlight.common: headers.go:55 omitting timezone header because: unable to find timezone for UTC (0)
So looks like the Go code is having trouble getting the timezone on Android.
There was a problem hiding this comment.
Here's the bug - golang/go#20455
I'll work on a workaround.
There was a problem hiding this comment.
There was a problem hiding this comment.
I see. If we always reset the data at midnight local time, on a specific day (depends on whether we reset the data daily, weekly or monthly), then why don't we just return the exact date instead of ttlSeconds?
There was a problem hiding this comment.
In hindsight that might have been better. The implementation actually uses Redis TTL so what's stored is actually an expiration in seconds, not a date. It has to be converted to a date somewhere, so I chose to let the client do it rather than the server.
True enough, still seems safest to evaluate this at construction time just in case. |
…se short format for date and time
oxtoacart
force-pushed
the
ox/issue4656
branch
from
db178ed to
6e86d6e
Compare
|
Thanks @markhor-sarwar ! |
This corrects a few issues I found with #20.
This PR addresses those issues. After this PR, you can remove the
convertTTSToDateTimeStringfromUtils.javain mobilesdk.