Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

weekly (and monthly?) Habit counters are resetting too often #8570

Closed
Alys opened this issue Mar 14, 2017 · 36 comments · Fixed by #8749
Closed

weekly (and monthly?) Habit counters are resetting too often #8570

Alys opened this issue Mar 14, 2017 · 36 comments · Fixed by #8749

Comments

@Alys
Copy link
Contributor

Alys commented Mar 14, 2017

The new Habit counters added in PR #8198 have a setting for resetting either each day or week or month. Currently, the weekly counters are always resetting each day, regardless of what setting is chosen. There is some evidence that monthly counters might sometimes also be resetting too soon but that's not certain (see one of the reports below).

I tested overnight by having a Habit set to each of the reset frequencies, with clicks recorded in each of their counters. The daily and weekly Habits were reset this morning and the monthly one was not. The weekly one should not have been reset. I've copied other people's reports from the Report a Bug guild at the bottom of this post.

Recent releases after the counters went live have made changes related to dates and cron, so it's not impossible that this feature broke after going live.
https://github.com/HabitRPG/habitica/releases/tag/v3.79.2 https://github.com/HabitRPG/habitica/releases/tag/v3.79.1
One of the reports from the Bug guild hints at that: "Mine (weekly) were saving fine til Sunday reset then I noticed they reset again this AM". Those two releases happened around Sunday, depending on time zone.

Worryingly though, the automatic tests aren't picking up the bug, even though I can see that there are tests for this behaviour.

I'm marking this as high priority because the counters are a solution to several common requests from users for better tracking of Habits and it would be good to get the resets working again soon.

@shalott If you have any ideas about what might be causing the resets to not work, we'd love to hear from you!

From the Report a Bug guild:

apolwapol (26677250-949e-4f84-b22b-3815f7ed5d5d):
Hi, I set the habit counter to reset weekly, but it still resets daily.
-- 18 hours ago

Yayef (23d6c69b-b1f6-4f02-940c-c5417ef3e1a1):
Hello. Same issue as @apolwapol : all my habits counters (weekly or monthly) reset daily.
-- 15 hours ago

M. Hostile (bea18590-b955-4c2b-9005-ec714fbcdbaa):
Hi! I like the new feature of habit counters but there seems to be a bug. My weekly counters reset every day. Monthly counters work fine. Thanks for your work.
-- 12 hours ago

joedragons (66902097-c79b-496f-acd9-b78cc5867b28):
Re habit counters: Mine (weekly) were saving fine til Sunday reset then I noticed they reset again this AM, in case that helps. Could be the first one works then after that gets off. Or daylight savings time (if you respect that=)). Or something changed around then? I just assumed they reset Sunday, maybe that's not true=) I turned them weekly the first day they were announced if that helps. Appreciate the investigation.
-- 9 hours ago

@SabreCat
Copy link
Member

I think weekly counters reset on Mondays. Perhaps that caused some confusion? I'll set one of mine to weekly and see if it persists through tomorrow.

@Alys
Copy link
Contributor Author

Alys commented Mar 14, 2017

Today is Wednesday for me, so the weekly shouldn't have reset today.

@SabreCat
Copy link
Member

OK, got it!

@joedragons
Copy link

Thanks team. If I can provide any more info, let me know. When I said Sunday reset I meant overnight Sunday to Monday AM (whenever it reset then, I assumed with my cron).

@Alys
Copy link
Contributor Author

Alys commented Mar 14, 2017

@joedragons So the weekly counter reset on Monday morning? That is expected. However from your comment it sounds like they also reset again after that - "this AM" - was that Tuesday or Wednesday morning?

@shalott
Copy link
Contributor

shalott commented Mar 14, 2017

Will attempt to reproduce the error overnight tonight in production and will take a look at the code tomorrow if I can! But it does sound like maybe the release happening on Sunday caused confusion for people since their weekly counters would have appeared to reset the next day, even though actually they were resetting on their Sunday->Monday cron.

@joedragons
Copy link

joedragons commented Mar 15, 2017

@Alys It reset for me both overnight Monday and today (Tuesday). So between bed and morning (also when my cron is set to run) both days. Sorry it's kind of confusing to explain especially because I am not sure exactly when the reset happened because I was asleep=)

So (pretend) example, if I went to bed at 11PM and got up at 7AM every day, it'd be:
Sunday 11PM(tallies are set) - Monday 3AMcron(may not matter, FYI) - Monday 7AM(tallies were reset)
and again
Monday 11PM(tallies are set) - Tuesday 3AMcron(may not matter, FYI) - Tuesday 7AM(tallies were reset)

@Alys
Copy link
Contributor Author

Alys commented Mar 15, 2017

@joedragons Thanks for those details! It's clear to us and that's useful to know.

For your information if you're curious, the Habit counters are reset by cron.

Also FYI, cron probably isn't running at exactly 3am. I'm assuming 3am is your Custom Day Start time, which means that it's the earliest possible time that cron can run. Cron actually runs at the exact time that you first use Habitica each day after your Custom Day Start time. So if you're still using Habitica at 3am, cron will run then, but more normally if you first use Habitica at 7am, then cron will run at 7am. (The situation can be slightly more complex if you use third-party tools that take automatic actions on your behalf in Habitica or if you leave the website open in your browser and it reloads itself automatically - each of those things count as you using Habitica, so cron will run when it happens, even though you might be asleep.)

@Yayef
Copy link

Yayef commented Mar 15, 2017

Some more information that might help: the first time I set weekly and monthly counters for habits was on Monday. On Tuesday, all counters (daily, weekly and monthly) were resetted to zero. Today (Wednesday), all counters are correct, there were not resetted tonight.

@joedragons
Copy link

@Alys Cool that's nice to know, I was curious=) So both times my computer was off, so per your explaination it would have been first thing in the morning run.
Like @Yayef this morning (Wednesday) my counters were ok. My computer was off again, there was no noteworthy difference to me between this morning and the last two.

@shalott
Copy link
Contributor

shalott commented Mar 15, 2017

My weekly counters did not reset tonight so I was unable to reproduce yet, but it occurs to me that the cron reset might be happening Monday->Tuesday instead of Sunday->Monday, and possibly only for some users?

The code, in cron.js, checks whether the day of week of the cron activity is Monday (ie, getDay() returns 1):

    let thatDay = moment(now).subtract({days: i}).toDate();
    if (thatDay.getDay() === 1) {
      resetWeekly = true;
    }
    if (thatDay.getDate() === 1) {
      resetMonthly = true;
    }

So I'm wondering now, is getDay() localized to the user's location by habitica? That is, in the US, the first day of the week is Sunday. However, in much of the world, the first day of the week is Monday. I would assume the value to be whatever the server time is, but possibly not. If some people continue to report their weekly cron reset happening Monday->Tuesday and others have it happening Sunday->Monday, this would probably be what's going on. I am not quite sure how to fix this yet but if that turns out to be the issue, will take a stab at it.

Alternatively, if the cron value is checking the day BEFORE the cron ran (although that doesn't seem to me to be what's happening, since when daysMissed = 0 the day should be the current day), then everyone's weekly counters would be resetting over Monday->Tuesday. If everyone reports weekly cron reset happening then, that's the issue (in which case the fix will be easy, just to switch getDay() === 1 to getDay() === 0 in the above code).

@SabreCat
Copy link
Member

I set up the habit in the Take This challenge to reset weekly, and it did not reset for me this morning (Wednesday).

@shalott, the code above doesn't take into account custom day start times. So cron could be running for Monday morning when the time moment() returns is Sunday (CDS 10PM or something). This suggests to me that we need a global function for determining the effective day of the week and month, taking into account when cron actually runs and user time zone. We had some trouble with this in @TheHollidayInn's draft of complex Daily repeat patterns. That said, this is a bit of a digression--I'm not convinced it'd actually cause incorrect resets in most of the cases being reported.

@Alys
Copy link
Contributor Author

Alys commented Mar 15, 2017

My weekly counter reset yesterday (Wednesday morning in my time zone). It did not reset this morning. The monthly counter reset on neither of those days.

My time zone is UTC+10 and my CDS is 2.

@Alys
Copy link
Contributor Author

Alys commented Mar 16, 2017

My weekly and monthly counters did not reset this morning (Friday).

Another report from the Bug guild, from Shiroi Chaika (60e74c6d-4919-46cb-b87f-bc8edd3dd5b7; CDS 0, UTC-4):
"I changed a number of my habits to reset weekly as soon as Bailey's news was announced, and but they've still been resetting every day. No data on monthly resets. No other problems with my Habitica and no other changes that I've made. Thanks!"

@shalott
Copy link
Contributor

shalott commented Mar 17, 2017 via email

@Alys
Copy link
Contributor Author

Alys commented Mar 17, 2017

@shalott yes, I can do that easily, and for each of the people who've reported it, I've checked that they do have habits set to weekly (I've been doing that without looking at the titles or notes for the tasks).

Tonight if I can make time or tomorrow at the latest, I'll set up an automatic process to collect Habit frequency and counter data (without any personal data about the contents of the tasks) so we can watch how they change.

@Alys
Copy link
Contributor Author

Alys commented Mar 19, 2017

My weekly Habit counters haven't reset over the past few days, including Sunday (which was the correct behaviour).

My weekly counters also didn't reset today (Monday), which is when they should reset. I'm at UTC+10 and it's still Sunday in UTC. I'll report again tomorrow.

My monthly counter hasn't reset at all (correct).

@shalott
Copy link
Contributor

shalott commented Mar 19, 2017

Alys, is there some way you can check what the result is of just this call on the server right now:

moment(now).toDate().getDay()

@joedragons
Copy link

Not sure how much continued info you want here esp if @Alys is doing account monitoring but my counters reset overnight (Sunday to Monday) as my above comment described and I expected better understanding how the feature works. I am (maybe pathetically) excited to see if they reset again tomorrow and there's repeatable behavior for me;)

@shalott
Copy link
Contributor

shalott commented Mar 20, 2017 via email

@Alys
Copy link
Contributor Author

Alys commented Mar 20, 2017

My weekly Habit counters reset this morning (Tuesday UTC+10). If my experience last week repeats, they'll also reset tomorrow morning so I'll post again then. My weekly Habits had been created last Tuesday so in case that affects the reset, I've created a new weekly Habit today, as well as having the one that I've been using for a week.

@joedragons This is almost certainly a time zone-dependent problem, so your input is definitely valuable!

@shalott I was afk when you posted your request yesterday. I believe the server runs on UTC time. You can see how cron handles the user's time zone in the sanitizeOptions function.

@Yayef
Copy link

Yayef commented Mar 21, 2017

My weekley habits counters reset yesterday (Monday UTC + 1) AND today (Tuesday UTC + 1). As far as I know, my monthly habit counter did not reset.

@joedragons
Copy link

joedragons commented Mar 21, 2017

My counters reset overnight (Monday to Tuesday, also UTC-5) for the second week. Seems consistent on my account to me!=) I'll only post outliers or new info from now on unless repeated info is desired ("Yes, it happened again"). Thanks for all your work!

PS: It randomly dawned on me during lunch that I wonder if no-one has seen bad behavior for monthlies because it hasn't been a month yet. I'd watch April 2nd:P

@Alys Alys changed the title weekly (and monhtly?) Habit counters are resetting every day weekly (and monthly?) Habit counters are resetting every day Mar 21, 2017
@Alys
Copy link
Contributor Author

Alys commented Mar 21, 2017

All my weekly counters reset again this morning (Wednesday) as well as yesterday (Tuesday), the same as last week. They haven't reset on any other days of the week. My monthly counters still haven't reset since this issue was logged.

It's looking like that's the standard behaviour, although the exact days on which it happens can very based on time zone (Monday and Tuesday OR Tuesday and Wednesday).

There's also this report from Quilynn (f36d8801-177f-4a86-936e-35aead16c9ca) in the Report a Bug guild:
"I've been having issues both this week and last with the weekly habit counter resetting on Tuesday morning. I KNOW (and the "data display tool confirms") that I checked off a habit yesterday, and today the count was at zero. It's set to reset weekly, I've checked many times. For most of the week it doesn't reset. I can't remember for sure if it reset Monday too but I think it did (so BOTH Monday and Tuesday)
Has anybody had the same experience? My cron is set to 4am, that might be related? (But i don't know how 4am tuesday is confused as Monday)."

@shalott
Copy link
Contributor

shalott commented Mar 21, 2017

Yep, mine reset both days as well--puzzling! I should be able to take a look at the code tomorrow and try and get in a fix.

@paglias
Copy link
Contributor

paglias commented Mar 22, 2017

Did a very quick investigation, @shalott you might find this useful:

{now} = sanitizeOptions({now, dayStart: user.preferences.dayStart, timezoneOffset: user.preferences.timezoneOffset}) 
  • not related to this issue in particular but if you work on a fix can you also include counterUp and counterDown to the list here, users should not be able to update them manually

@shalott
Copy link
Contributor

shalott commented Mar 22, 2017

Thanks, @paglias, I can easily understand why the reset would happen at the WRONG time due to user timezone issues, the thing that is baffling me is why it happens TWICE--presumably the cron should be wrong the same way each time, no?

Anyway, first going to try and get a test to reproduce the issue and then that should clarify!

@paglias
Copy link
Contributor

paglias commented Mar 22, 2017 via email

@quilynn
Copy link

quilynn commented Mar 28, 2017

Adding my experience from this week, Alys copy-pasted my message from the Report a Bug guild above (I'm Quilynn)

This week I definitely had my habits reset on the mornings of BOTH Monday AND Tuesday for my habits marked to reset weekly. So as the cron triggered on both Monday and Tuesday it reset. My reset time is at the default, 12 AM, (UTC -6:00). They aren't resetting every day, only Mon+Tue.

@SabreCat
Copy link
Member

OK, yup. I had my weekly Challenge task reset today, too. It didn't reset yesterday because I was in the Inn.

@Alys Alys changed the title weekly (and monthly?) Habit counters are resetting every day weekly (and monthly?) Habit counters are resetting too often May 17, 2017
@joe-salomon2
Copy link

@Alys, can I take on this task?

@Alys
Copy link
Contributor Author

Alys commented May 21, 2017

@joe-salomon Please do! Post here if you have questions or need advice about anything!

SabreCat pushed a commit that referenced this issue Jul 18, 2017
)

* For habit reset logic, changed day check calculation to use user’s timezone instead of server time.
Added unit tests to check following cases:
- Weekly habit reset: Server tz is Sunday, User tz is Monday
- Weekly habit reset: Server tz is Monday, User tz is Sunday
- Monthly habit reset: Server tz is 1st of month, User tz is 2nd of month
- Monthly habit reset: Server tz is end of prev month, User tz is 1st of month

* use moment().zone() instead of utcOffset()

* typo

* Fixed check for daysMissed, added logic for CDS
Added test for CDS, fixed previous tests
@joedragons
Copy link

Is this fix live (on the website)? I saw the same behavior this AM. I still don't have a good handle over when merged code goes live, I suppose. Just wanted to make sure there's no edge cases, really.

@SabreCat
Copy link
Member

SabreCat commented Jul 25, 2017

@joedragons Not live yet! It's in a week of staging testing before we send it out to everyone. The best way to see what's actually out on the production site is the releases page.

@joedragons
Copy link

Thanks @SabreCat ; I'll monitor that page from now on=)

@joedragons
Copy link

This appears fixed to me, btw, thanks all for their efforts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants