Current Streak update so specific times aren't missed #13558
Conversation
BerkeleyTrue
added
the
status: waiting review
| @@ -2,7 +2,7 @@ import _ from 'lodash'; | |||
| import moment from 'moment-timezone'; | |||
| import { dayCount } from '../utils/date-utils'; | |||
|
|
|||
| const daysBetween = 1.5; | |||
| const daysBetween = 2; | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| @@ -16,7 +16,7 @@ export function prepUniqueDays(cals, tz = 'UTC') { | |||
| export function calcCurrentStreak(cals, tz = 'UTC') { | |||
|
|
|||
| let prev = _.last(cals); | |||
| if (moment().tz(tz).startOf('day').diff(prev, 'days') > daysBetween) { | |||
| if (moment().tz(tz).diff(prev, 'days') > daysBetween) { | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| 4, | ||
| 'should return 4 when there is a break in UTC (but no break in PST)' | ||
| 16, | ||
| 'should return 16 when there is a break in UTC (but no break in PST)' |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@raisedadead will probably want a lot of eyes on this because it affects a very visible feature. Also it was tough to test locally because I couldn't force change my local time to see if streaks were continuing... |
|
@donofriov thanks a lot for this pull request, and apologies for the delay in getting back. I have been away for a while now. @BerkeleyTrue the code looks fine to me, but there is no real way to confirm without sending this change out to the wild, if you think this is okay, we should test this on beta and keep under observation? /cc @QuincyLarson I would seek your advice on this as well. |
|
@raisedadead I think one way we can make sure we didn't accidentally make it less inclusive is to ship this and see what happens to a profile like @revisualize's: https://www.freecodecamp.com/revisualize As long as his streak is still intact, we'll know it's at least as inclusive as before (and we want to err on the side of inclusivity and streak generosity). |
|
Please don't merge this as is, I just found a bug with longest streak. I'm able to force some data locally, will submit an update today. |
raisedadead
added
the
status: blocked
BerkeleyTrue
removed
the
status: waiting review
|
@donofriov Any updates on this? |
|
Hi @donofriov I am closing this, please re-open when you are ready. Thanks a lot for the hard work. |
raisedadead
removed
the
status: blocked
Pre-Submission Checklist
stagingbranch of freeCodeCamp.fix/,feature/, ortranslate/(e.g.fix/signin-issue)npm test. Usegit commit --amendto amend any fixes.Type of Change
Checklist:
Description
I made a few updates to how the current streak works to cover all cases close to the "next day" where a streak wasn't continuing even though it should have.