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

Yesterdalies #8717

Closed
wants to merge 36 commits into
base: develop
from

Conversation

Projects
None yet
6 participants
@TheHollidayInn
Contributor

TheHollidayInn commented May 4, 2017

Changes

This adds Yesterdailies - which allows users to confirm if they have complete dalies that were missed the previous day.

The flow is as follows:

  • A daily is defaulted to a "Yesterdaily"
  • After cron runs a day after the daily is due, the daily is added to the user's yesterDailies list
  • They are then presented with a modal to either score the dailys or admit defeat
  • Then, the dalies are aged

Also, the user can turn of the yesterdaily option.
yesterdailies


UUID:

@Alys

This comment has been minimized.

Show comment
Hide comment
@Alys

Alys May 4, 2017

Contributor

Could you please add a screenshot of the option to turn this off in the taek edit screen?

Contributor

Alys commented May 4, 2017

Could you please add a screenshot of the option to turn this off in the taek edit screen?

@MathWhiz

This comment has been minimized.

Show comment
Hide comment
@MathWhiz

MathWhiz May 5, 2017

Contributor

Love the name!

Contributor

MathWhiz commented May 5, 2017

Love the name!

Show outdated Hide outdated gulp/gulp-tests.js
Show outdated Hide outdated test/helpers/api-integration/v3/object-generators.js
@@ -197,5 +197,7 @@
"online": "online",
"onlineCount": "<%= count %> online",
"loading": "Loading...",
"userIdRequired": "User ID is required"
"userIdRequired": "User ID is required",

This comment has been minimized.

@paglias

paglias May 7, 2017

Contributor

quite sure we have this message somewhere else and anyway it should be an ApiMessage not a locale string

@paglias

paglias May 7, 2017

Contributor

quite sure we have this message somewhere else and anyway it should be an ApiMessage not a locale string

This comment has been minimized.

@TheHollidayInn

TheHollidayInn May 9, 2017

Contributor

Yea, I just added the comma here. This is not new.

@TheHollidayInn

TheHollidayInn May 9, 2017

Contributor

Yea, I just added the comma here. This is not new.

Show outdated Hide outdated website/server/controllers/api-v3/tasks.js
let taskIds = user.yesterDailies;
let dailies = await Tasks.Task.find({_id: taskIds, type: 'daily'}).exec();
let daysMissed = 1; // I believe we only handle one day damage now. Maybe multi can be for hard mode?

This comment has been minimized.

@paglias

paglias May 7, 2017

Contributor

@Alys and @SabreCat can confirm but I think cron handles multiple days damage

@paglias

paglias May 7, 2017

Contributor

@Alys and @SabreCat can confirm but I think cron handles multiple days damage

This comment has been minimized.

@Alys

Alys May 7, 2017

Contributor

Cron has support for multiple days to be implemented in future. Ideally, daysMissed would be passed through to this new code as a variable rather than as a fixed 1

@Alys

Alys May 7, 2017

Contributor

Cron has support for multiple days to be implemented in future. Ideally, daysMissed would be passed through to this new code as a variable rather than as a fixed 1

This comment has been minimized.

@TheHollidayInn

TheHollidayInn May 8, 2017

Contributor

Yea, cron was set to always handle one day missed, but if we plan on using it in the future, that's fine.

@TheHollidayInn

TheHollidayInn May 8, 2017

Contributor

Yea, cron was set to always handle one day missed, but if we plan on using it in the future, that's fine.

This comment has been minimized.

@TheHollidayInn

TheHollidayInn May 9, 2017

Contributor

Actually, looking at this again, when we support multiple days, we just adjust this variable. The ageDailies function takes the days missed variable.

@TheHollidayInn

TheHollidayInn May 9, 2017

Contributor

Actually, looking at this again, when we support multiple days, we just adjust this variable. The ageDailies function takes the days missed variable.

* @apiSuccess {Object} data An empty object
*
*/
api.ageDailies = {

This comment has been minimized.

@paglias

paglias May 7, 2017

Contributor

this route can be called any number of times a day?

@paglias

paglias May 7, 2017

Contributor

this route can be called any number of times a day?

This comment has been minimized.

@TheHollidayInn

TheHollidayInn May 8, 2017

Contributor

Yea, but the first time it is called the user will no longer have yesterdailies

@TheHollidayInn

TheHollidayInn May 8, 2017

Contributor

Yea, but the first time it is called the user will no longer have yesterdailies

This comment has been minimized.

@paglias

paglias May 9, 2017

Contributor

Ok, so nothing happens if you call it multiple times in a day?

@paglias

paglias May 9, 2017

Contributor

Ok, so nothing happens if you call it multiple times in a day?

This comment has been minimized.

@TheHollidayInn

TheHollidayInn May 9, 2017

Contributor

Exactly

@TheHollidayInn

TheHollidayInn May 9, 2017

Contributor

Exactly

Show outdated Hide outdated website/server/libs/taskManager.js
@Alys

This comment has been minimized.

Show comment
Hide comment
@Alys

Alys May 9, 2017

Contributor

How will this work when cron is being attempted in two places at once?

For example, I leave work at night with Habitica still open in my browser. It reloads itself after 6 hours and if that's past midnight then cron is triggered, and the modal showing the incomplete Dailies appears. However I'm not at work to interact with the modal so it (I assume) stays open. Several hours after that, I wake up and open Habitica in my browser at home. What happens then?

Does the modal with incomplete Dailies also appear on my home browser and can I interact with the modal as normal and then allow cron to finish?

Or am I not able to see the modal at home because it's already open at work?

If I am able to interact with the modal at home and cron finishes there, what happens when I get to work and see the modal still open?
If I try to interact with the modal again (e.g., mark yesterday's Dailies as complete again), what happens?
Will it cause some of cron's processes to run again? Or will it refuse to let me use the modal, with a helpful error message?

If I recognise that I shouldn't interact with the modal at work, can I cancel the modal without interacting with it?
If there is an option to cancel it, what happens if I try to use that option at home when I should not be able to cancel it?

Contributor

Alys commented May 9, 2017

How will this work when cron is being attempted in two places at once?

For example, I leave work at night with Habitica still open in my browser. It reloads itself after 6 hours and if that's past midnight then cron is triggered, and the modal showing the incomplete Dailies appears. However I'm not at work to interact with the modal so it (I assume) stays open. Several hours after that, I wake up and open Habitica in my browser at home. What happens then?

Does the modal with incomplete Dailies also appear on my home browser and can I interact with the modal as normal and then allow cron to finish?

Or am I not able to see the modal at home because it's already open at work?

If I am able to interact with the modal at home and cron finishes there, what happens when I get to work and see the modal still open?
If I try to interact with the modal again (e.g., mark yesterday's Dailies as complete again), what happens?
Will it cause some of cron's processes to run again? Or will it refuse to let me use the modal, with a helpful error message?

If I recognise that I shouldn't interact with the modal at work, can I cancel the modal without interacting with it?
If there is an option to cancel it, what happens if I try to use that option at home when I should not be able to cancel it?

@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn May 9, 2017

Contributor

So, basically you just have to think of any issue like this with respect to the state.

  1. If you have yesterdalies in the user array, the model will appear and you can interact with it.
  2. If you have no yesterdalies in your array on the server, but the client is showing the modal, you can close the modal and no affect will happen (per paglias question above).

The server only ages based on what is stored in the database. So, incorrect syncing on the client will not affect this. And the client, whenever updated, will see the modal. There isn't a timer or anything - so the modal is not a one time thing. The modal shows until you have addressed the yesterdalies.

Contributor

TheHollidayInn commented May 9, 2017

So, basically you just have to think of any issue like this with respect to the state.

  1. If you have yesterdalies in the user array, the model will appear and you can interact with it.
  2. If you have no yesterdalies in your array on the server, but the client is showing the modal, you can close the modal and no affect will happen (per paglias question above).

The server only ages based on what is stored in the database. So, incorrect syncing on the client will not affect this. And the client, whenever updated, will see the modal. There isn't a timer or anything - so the modal is not a one time thing. The modal shows until you have addressed the yesterdalies.

@Alys

This comment has been minimized.

Show comment
Hide comment
@Alys

Alys May 10, 2017

Contributor

And yesterdalies remain in the user array until after you've interacted with the modal, right?

So in the situation I described, the user IS able to see the modal at home, even though it's already showing at work, and cron WILL happen correctly at home?

And then when they get to work, they can click through the modal and it will disappear without making any further changes to the user's data?

Unrelated to that, can you add a screenshot of the option to turn off the yesterdaily feature in the task edit screen?

Contributor

Alys commented May 10, 2017

And yesterdalies remain in the user array until after you've interacted with the modal, right?

So in the situation I described, the user IS able to see the modal at home, even though it's already showing at work, and cron WILL happen correctly at home?

And then when they get to work, they can click through the modal and it will disappear without making any further changes to the user's data?

Unrelated to that, can you add a screenshot of the option to turn off the yesterdaily feature in the task edit screen?

@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn May 15, 2017

Contributor

yes, yes, and yes.

Sorry. I have missed this twice:

yesterdaily

Contributor

TheHollidayInn commented May 15, 2017

yes, yes, and yes.

Sorry. I have missed this twice:

yesterdaily

@SabreCat

This comment has been minimized.

Show comment
Hide comment
@SabreCat

SabreCat May 15, 2017

Member

We should probably come up with an alternative description there--"Yesterdailies" is something of an internal development term for the feature, I don't know that it was intended to be end-user-facing. Maybe something like "Prompt to check off if missed"?

Member

SabreCat commented May 15, 2017

We should probably come up with an alternative description there--"Yesterdailies" is something of an internal development term for the feature, I don't know that it was intended to be end-user-facing. Maybe something like "Prompt to check off if missed"?

@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn May 15, 2017

Contributor

Yea, we definitely need different wording.

Contributor

TheHollidayInn commented May 15, 2017

Yea, we definitely need different wording.

@Alys

This comment has been minimized.

Show comment
Hide comment
@Alys

Alys May 20, 2017

Contributor

I've done some testing on my local install and put some points below. When I was most of the way through writing them, I realised that some of them are probably redundant. I think several are symptoms of the same problem: the yesterdailies code seems to be happening at the wrong point in cron. Instead of happening at the very start before any processing of the Dailies, quest, etc has been done, it's happening after most of the processing. That's producing undesirable effects such as the yesterdailies not becoming unticked when cron finishes.

  • In the modal I get when cron starts, when I mark a Daily complete, there's no visual change to the Daily -- it still looks incomplete. Ideally it would become grey and have a tick mark in its checkbox, or possibly be removed from the list displayed in the modal (although I think grey and ticked would be better). I think this is important because if the user doesn't realise that the Daily is now marked as complete, they'll tick it again and that will cause it to be incomplete, leading to confusion and unfair damage when cron finishes.
  • After cron had finished, the Dailies that I had ticked in the modal were still marked as complete. They should be incomplete because in the yesterdailies modal I was ticking them for yesterday, not for today.
  • After cron had finished, the extra boss damage that I'd gained from ticking Dailies in the modal was not applied to the boss - it was left as pending damage for today. It should have been applied because I was marking the Dailies as complete for yesterday.
  • Related to the above point: If the user is in a quest and has pending damage showing in the notification icon in the header, then when the yesterdailies modal pops up, the damage icon disappears - i.e., it looks like at least some of the cron functions are completed before the yesterdailies modal is actioned by the user.
  • [Part fixed, part copied below] If the Daily comes from a Challenge or a Group Plan, it's not possible for the user to change whether it's a yesterdaily or not - I think it should be possible. All Challenge and Group Plan Dailies should start as yesterdailies and the owner/leader should not be able to change that, because that's the safe option that prevents the user from taking unfair damage. The user should be in control of whether it's a yesterdaily because the user knows best whether they need to turn off the temptation to cheat or leave on the protection if they often suffer from sync errors or internet outages.
  • If the Daily has a checklist, the icon that displays how many checklist items have been marked completed is missing the first number. I.e., in the screenshot below, /4 should be 3/4
    image
  • [copied below] If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers. The correct behaviour is that when the Daily is not complete but some checklist items are ticked, then the user takes less damage. If all checklist items are complete, the user takes no damage from that Daily (even when the Daily itself is not marked complete).
  • If the user leaves some yesterdaily Dailies incomplete, the Dailies do not have their streaks removed and their value does not decrease (their colour does not move towards red). If any of their checklist items had been ticked, those items do not become unticked.
  • If the user leaves some NON-yesterdaily Dailies incomplete, they do lose their streaks and they do redden but if any of their checklist items had been ticked, those items do not become unticked.
  • If a Group Plan yesterDaily that requires approval has been marked complete but approval has not yet been given, it will appear in the yesterdailies modal. It should not appear because the user cannot mark it complete again.
  • [copied below] If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, you see the normal notification saying that approval has been requested, but the "approval requested" text does not appear on the Daily. It should appear so that the user knows they don't need to mark it complete again (similar situation to the first point above, about how there should be a visual change to the Dailies after completion)
  • If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, a second yesterdailies modal pops up above the first. I think this happens under only some circumstances (I'm not sure which ones) because earlier in my testing I was not getting this behaviour but was getting the behaviour from the next point instead.
  • If a Group Plan Daily is marked as complete in the yesterdailies modal, you get the expected "approval requested" notification but when you click the "That's All" button, you get the error message Cast to Number failed for value "NaN" at path "party.quest.progress.down" and the yesterdailies modal pops back up. If you try to dismiss it again, it pops up again, and that keeps repeating. This is not consistent though - see the point immediately above.
  • When a Daily is created by the user in their personal task list, if they immediately open the Daily for editing, the yesterdaily checkbox is not ticked. However the Daily is actually a yesterdaily (confirmed by looking in the database). If you sync and then edit the Daily, the yesterdaily checkbox is ticked correctly. This is important to fix because users often edit the attributes of new tasks (especially those who use "Open new tasks in edit mode"), and it's misleading if the new Daily looks like it's not a yesterdaily when it is. It will lead to bug reports from users who don't want to use the yesterdaily feature.
  • In two of my early tests, the yesterdailies modal appeared as in the screenshot below. I haven't yet determined what caused that but we should watch for it happening again. I'll try to do more investigation on it in future tests.
    image
Contributor

Alys commented May 20, 2017

I've done some testing on my local install and put some points below. When I was most of the way through writing them, I realised that some of them are probably redundant. I think several are symptoms of the same problem: the yesterdailies code seems to be happening at the wrong point in cron. Instead of happening at the very start before any processing of the Dailies, quest, etc has been done, it's happening after most of the processing. That's producing undesirable effects such as the yesterdailies not becoming unticked when cron finishes.

  • In the modal I get when cron starts, when I mark a Daily complete, there's no visual change to the Daily -- it still looks incomplete. Ideally it would become grey and have a tick mark in its checkbox, or possibly be removed from the list displayed in the modal (although I think grey and ticked would be better). I think this is important because if the user doesn't realise that the Daily is now marked as complete, they'll tick it again and that will cause it to be incomplete, leading to confusion and unfair damage when cron finishes.
  • After cron had finished, the Dailies that I had ticked in the modal were still marked as complete. They should be incomplete because in the yesterdailies modal I was ticking them for yesterday, not for today.
  • After cron had finished, the extra boss damage that I'd gained from ticking Dailies in the modal was not applied to the boss - it was left as pending damage for today. It should have been applied because I was marking the Dailies as complete for yesterday.
  • Related to the above point: If the user is in a quest and has pending damage showing in the notification icon in the header, then when the yesterdailies modal pops up, the damage icon disappears - i.e., it looks like at least some of the cron functions are completed before the yesterdailies modal is actioned by the user.
  • [Part fixed, part copied below] If the Daily comes from a Challenge or a Group Plan, it's not possible for the user to change whether it's a yesterdaily or not - I think it should be possible. All Challenge and Group Plan Dailies should start as yesterdailies and the owner/leader should not be able to change that, because that's the safe option that prevents the user from taking unfair damage. The user should be in control of whether it's a yesterdaily because the user knows best whether they need to turn off the temptation to cheat or leave on the protection if they often suffer from sync errors or internet outages.
  • If the Daily has a checklist, the icon that displays how many checklist items have been marked completed is missing the first number. I.e., in the screenshot below, /4 should be 3/4
    image
  • [copied below] If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers. The correct behaviour is that when the Daily is not complete but some checklist items are ticked, then the user takes less damage. If all checklist items are complete, the user takes no damage from that Daily (even when the Daily itself is not marked complete).
  • If the user leaves some yesterdaily Dailies incomplete, the Dailies do not have their streaks removed and their value does not decrease (their colour does not move towards red). If any of their checklist items had been ticked, those items do not become unticked.
  • If the user leaves some NON-yesterdaily Dailies incomplete, they do lose their streaks and they do redden but if any of their checklist items had been ticked, those items do not become unticked.
  • If a Group Plan yesterDaily that requires approval has been marked complete but approval has not yet been given, it will appear in the yesterdailies modal. It should not appear because the user cannot mark it complete again.
  • [copied below] If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, you see the normal notification saying that approval has been requested, but the "approval requested" text does not appear on the Daily. It should appear so that the user knows they don't need to mark it complete again (similar situation to the first point above, about how there should be a visual change to the Dailies after completion)
  • If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, a second yesterdailies modal pops up above the first. I think this happens under only some circumstances (I'm not sure which ones) because earlier in my testing I was not getting this behaviour but was getting the behaviour from the next point instead.
  • If a Group Plan Daily is marked as complete in the yesterdailies modal, you get the expected "approval requested" notification but when you click the "That's All" button, you get the error message Cast to Number failed for value "NaN" at path "party.quest.progress.down" and the yesterdailies modal pops back up. If you try to dismiss it again, it pops up again, and that keeps repeating. This is not consistent though - see the point immediately above.
  • When a Daily is created by the user in their personal task list, if they immediately open the Daily for editing, the yesterdaily checkbox is not ticked. However the Daily is actually a yesterdaily (confirmed by looking in the database). If you sync and then edit the Daily, the yesterdaily checkbox is ticked correctly. This is important to fix because users often edit the attributes of new tasks (especially those who use "Open new tasks in edit mode"), and it's misleading if the new Daily looks like it's not a yesterdaily when it is. It will lead to bug reports from users who don't want to use the yesterdaily feature.
  • In two of my early tests, the yesterdailies modal appeared as in the screenshot below. I haven't yet determined what caused that but we should watch for it happening again. I'll try to do more investigation on it in future tests.
    image
@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn May 22, 2017

Contributor
  • Updated In the modal I get when cron starts, when I mark a Daily complete, there's no visual change to the Daily
  • Updated After cron had finished, the Dailies that I had ticked in the modal were still marked as complete.
  • Updated After cron had finished, the extra boss damage that I'd gained from ticking Dailies in the modal was not applied to the boss
  • I'm not exactly sure how to recreate or what this is Related to the above point: If the user is in a quest and has pending damage showing in the notification icon in the header, then when the yesterdailies modal pops up, the damage icon disappears
  • Updated If the Daily comes from a Challenge or a Group Plan, it's not possible for the user to change whether it's a yesterdaily or not
  • *Updated If the Daily has a checklist, the icon that displays how many checklist items have been marked completed is missing the first number. I.e., in the screenshot below, /4 should be 3/4
  • Could not reproduce. The score function is called the same and hasn't been changed. Which used checklist items. If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers.
  • Updated If the user leaves some yesterdaily Dailies incomplete, the Dailies do not have their streaks removed and their value does not decrease (their colour does not move towards red).
  • Updated If the user leaves some NON-yesterdaily Dailies incomplete, they do lose their streaks and they do redden but if any of their checklist items had been ticked, those items do not become unticked.
  • Updated If a Group Plan yesterDaily that requires approval has been marked complete but approval has not yet been given, it will appear in the yesterdailies modal. It should not appear because the user cannot mark it complete again.
  • UPdated If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, you see the normal notification saying that approval has been requested, but the "approval requested" text does not appear on the Daily. It should appear so that the user knows they don't need to mark it complete again (similar situation to the first point above, about how there should be a visual change to the Dailies after completion)
  • Updated If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, a second yesterdailies modal pops up above the first. I think this happens under only some circumstances (I'm not sure which ones) because earlier in my testing I was not getting this behaviour but was getting the behaviour from the next point instead.
  • Updated If a Group Plan Daily is marked as complete in the yesterdailies modal, you get the expected "approval requested" notification but when you click the "That's All" button, you get the error message
  • Updated When a Daily is created by the user in their personal task list, if they immediately open the Daily for editing, the yesterdaily checkbox is not ticked
  • Updated In two of my early tests, the yesterdailies modal appeared as in the screenshot below. I haven't yet determined what caused that but we should watch for it happening again. I'll try to do more investigation on it in future tests.
Contributor

TheHollidayInn commented May 22, 2017

  • Updated In the modal I get when cron starts, when I mark a Daily complete, there's no visual change to the Daily
  • Updated After cron had finished, the Dailies that I had ticked in the modal were still marked as complete.
  • Updated After cron had finished, the extra boss damage that I'd gained from ticking Dailies in the modal was not applied to the boss
  • I'm not exactly sure how to recreate or what this is Related to the above point: If the user is in a quest and has pending damage showing in the notification icon in the header, then when the yesterdailies modal pops up, the damage icon disappears
  • Updated If the Daily comes from a Challenge or a Group Plan, it's not possible for the user to change whether it's a yesterdaily or not
  • *Updated If the Daily has a checklist, the icon that displays how many checklist items have been marked completed is missing the first number. I.e., in the screenshot below, /4 should be 3/4
  • Could not reproduce. The score function is called the same and hasn't been changed. Which used checklist items. If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers.
  • Updated If the user leaves some yesterdaily Dailies incomplete, the Dailies do not have their streaks removed and their value does not decrease (their colour does not move towards red).
  • Updated If the user leaves some NON-yesterdaily Dailies incomplete, they do lose their streaks and they do redden but if any of their checklist items had been ticked, those items do not become unticked.
  • Updated If a Group Plan yesterDaily that requires approval has been marked complete but approval has not yet been given, it will appear in the yesterdailies modal. It should not appear because the user cannot mark it complete again.
  • UPdated If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, you see the normal notification saying that approval has been requested, but the "approval requested" text does not appear on the Daily. It should appear so that the user knows they don't need to mark it complete again (similar situation to the first point above, about how there should be a visual change to the Dailies after completion)
  • Updated If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, a second yesterdailies modal pops up above the first. I think this happens under only some circumstances (I'm not sure which ones) because earlier in my testing I was not getting this behaviour but was getting the behaviour from the next point instead.
  • Updated If a Group Plan Daily is marked as complete in the yesterdailies modal, you get the expected "approval requested" notification but when you click the "That's All" button, you get the error message
  • Updated When a Daily is created by the user in their personal task list, if they immediately open the Daily for editing, the yesterdaily checkbox is not ticked
  • Updated In two of my early tests, the yesterdailies modal appeared as in the screenshot below. I haven't yet determined what caused that but we should watch for it happening again. I'll try to do more investigation on it in future tests.
@SabreCat

This comment has been minimized.

Show comment
Hide comment
@SabreCat

SabreCat May 23, 2017

Member

I'm not exactly sure how to recreate or what this is Related to the above point: If the user is in a quest and has pending damage showing in the notification icon in the header, then when the yesterdailies modal pops up, the damage icon disappears

@Alys is referring to this icon:
image

To see it, start a quest and check off some tasks.

Member

SabreCat commented May 23, 2017

I'm not exactly sure how to recreate or what this is Related to the above point: If the user is in a quest and has pending damage showing in the notification icon in the header, then when the yesterdailies modal pops up, the damage icon disappears

@Alys is referring to this icon:
image

To see it, start a quest and check off some tasks.

@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn May 23, 2017

Contributor

Thanks! I messaged her on Slack and she showed me.

Contributor

TheHollidayInn commented May 23, 2017

Thanks! I messaged her on Slack and she showed me.

@Alys

This comment has been minimized.

Show comment
Hide comment
@Alys

Alys May 27, 2017

Contributor
  • On task edit screen, "Mark this as a Yesterdaily" could be changed to something like "Prompt to check off if missed"
  • I think that text would also benefit from an on-hover tip in the same way that "Task Alias" has a tip. We could use placeholder text for now until we work out what explanations are needed.
  • When I cron, the Record Yesterday's Activity modal usualy doesn't appear until I've reloaded the page.
  • A Group Plan leader or manager is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a member claims the task, it should be a yesterdaily by default).
  • A Challenge owner is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a participant joins the challenge, the Daily should be a yesterdaily by default).
  • If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers. The correct behaviour is that when the Daily is not complete but some checklist items are ticked, then the user takes less damage. If all checklist items are complete, the user should take no damage from that Daily (even when the Daily itself is not marked complete).
  • If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete, and then in the yesterdailies modal they mark the Daily itself as complete, the checklist items are not unticked (they should become unticked).
  • If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, the Daily becomes grey and ticked. It should remain coloured and unticked but with the "approval requested" text on it.
  • When the player is in a quest and has Dailies that they mark complete in the yesterdailies modal, two quest progress messages are produced in party chat instead of one (one message for the damage accumulated before the yesterdailies modal appears and one for the Dailies processed in the yesterdailies modal). Ideally a single message will be produced since all of that damage is for a single day.
  • When the player marks Dailies complete in the yesterdailies modal, they can get drops even if they already met their drop cap for the previous day (ideally that wouldn't happen because the tasks they're marking as complete are for the previous day and so shouldn't allow them to exceed that day's drop cap). The drops they get count towards the new day's drop cap (also not the ideal behaviour since it reduces one of the motivational features that encourages them to complete new tasks on the new day).
  • If the user marks a Daily as complete in the modal and then realises they ticked the wrong one, they'll try to untick it but they can't - the Daily stays ticked and grey. If they keey trying to untick it, they keep losing GP, XP, and MP for every click. The desired behaviour is that the first tick should complete it, the second tick should uncomplete it, a third tick should complete it again, etc.
Contributor

Alys commented May 27, 2017

  • On task edit screen, "Mark this as a Yesterdaily" could be changed to something like "Prompt to check off if missed"
  • I think that text would also benefit from an on-hover tip in the same way that "Task Alias" has a tip. We could use placeholder text for now until we work out what explanations are needed.
  • When I cron, the Record Yesterday's Activity modal usualy doesn't appear until I've reloaded the page.
  • A Group Plan leader or manager is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a member claims the task, it should be a yesterdaily by default).
  • A Challenge owner is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a participant joins the challenge, the Daily should be a yesterdaily by default).
  • If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers. The correct behaviour is that when the Daily is not complete but some checklist items are ticked, then the user takes less damage. If all checklist items are complete, the user should take no damage from that Daily (even when the Daily itself is not marked complete).
  • If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete, and then in the yesterdailies modal they mark the Daily itself as complete, the checklist items are not unticked (they should become unticked).
  • If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, the Daily becomes grey and ticked. It should remain coloured and unticked but with the "approval requested" text on it.
  • When the player is in a quest and has Dailies that they mark complete in the yesterdailies modal, two quest progress messages are produced in party chat instead of one (one message for the damage accumulated before the yesterdailies modal appears and one for the Dailies processed in the yesterdailies modal). Ideally a single message will be produced since all of that damage is for a single day.
  • When the player marks Dailies complete in the yesterdailies modal, they can get drops even if they already met their drop cap for the previous day (ideally that wouldn't happen because the tasks they're marking as complete are for the previous day and so shouldn't allow them to exceed that day's drop cap). The drops they get count towards the new day's drop cap (also not the ideal behaviour since it reduces one of the motivational features that encourages them to complete new tasks on the new day).
  • If the user marks a Daily as complete in the modal and then realises they ticked the wrong one, they'll try to untick it but they can't - the Daily stays ticked and grey. If they keey trying to untick it, they keep losing GP, XP, and MP for every click. The desired behaviour is that the first tick should complete it, the second tick should uncomplete it, a third tick should complete it again, etc.
@Alys

This comment has been minimized.

Show comment
Hide comment
@Alys

Alys May 27, 2017

Contributor
  • There's a bug that occurs if a user has the website open in two browser tabs or two different browsers - for example, at home and at work. The most troublesome aspects are highlighted in bold below.
    If the website is left open overnight on both browsers, the regular 6-hour reload will trigger cron in both.
    The first cron process runs just once, as you can see if there's a quest in progress - there's only one boss quest message at that point (we have protections in place to stop cron running twice).
    However the "Did you complete any of these?" modal appears on both browsers.
    The user will see it first at home in the morning and will tick the appropriate tasks then dismiss the modal.
    Habitica then processes all of the tasks from the yesterDailies array as intended (and a second quest progress message appears in party chat, as listed in one of the bugs above).
    The user then goes to work and sees the same "Did you complete any of these?" modal there too - there's no indication that the modal should no longer be used.
    The user could think that their actions at home weren't recorded, or could think that they must have forgotten to handle the modal at home.
    The user ticks the tasks again and the modal lets them do that. The user earns XP, etc from tasks that they'd already ticked at home, and that XP, etc remains in their account (it's not removed later after a sync).
    The user then dismisses the modal.
    Habitica then processes the tasks from the modal again.
    A third quest progress message appears in party chat, with damage to and from the boss being caused by the same tasks that resulted in the second quest progress message. The damage to the boss actually seems to be almost doubled for some reason, even when the same tasks are ticked on both modals. See the screenshot below -- all three quest progress messages are within a couple of minutes since I was doing this as a test but in the real scenario I described, the first message might be from say 2am when the browser reloads, the second might be from 6am when the user starts using Habitica at home, and the third from 9am at work.
    The tasks that were completed in the modal at work have remained completed in the user's tasks list - they are not uncompleted and ready to be used from scratch today.
    The user can uncomplete them manually, and then the user loses the XP, etc that they'd gained inappropriately, so that part is correct, but this will be confusing to many the users.

Similar behaviour will probably occur if the user has Habitica open in a browser somehere but first uses it in a mobile app, or if they use a third-party tool that

The ideal behaviour is that when the "Did you complete any of these?" modal appears in more than one tab or browser or device, it should only be actionable once. It should be impossible for the user to do anything on the second modal except close it.

image

"chat": [
    {
        "id": "c264c958-da79-4397-9f24-20eebac1edfc",
        "text": "`Canasta attacks Vice's Shade for 4.8 damage.` `Vice's Shade attacks party for 1.3 damage.`",
        "timestamp": 1495898346389,	// 	UTC: 2017-05-27 15:19:06
        "flagCount": 0
    },
    {
        "id": "a6f6f287-2c66-4069-a487-6607c2c7a3a9",
        "text": "`Canasta attacks Vice's Shade for 2.5 damage.` `Vice's Shade attacks party for 1.3 damage.`",
        "timestamp": 1495898339622,	// 	UTC: 2017-05-27 15:18:59
        "flagCount": 0
    },
    {
        "id": "197d79be-b64e-4e7e-82ec-8130090d0798",
        "text": "`Canasta attacks Vice's Shade for 20.7 damage.` `Vice's Shade attacks party for 7.9 damage.`",
        "timestamp": 1495898295840,	// 	UTC: 2017-05-27 15:18:15
        "flagCount": 0
    },
    {
        "id": "340b85e1-33ea-421e-aa8b-b353a3e42c1c",
        "text": "message before dual browser test",
        "timestamp": 1495898214878,	// 	UTC: 2017-05-27 15:16:54
        "flagCount": 0,
        "user": "Canasta"
    },
    {
        "id": "4c158b20-bb19-4727-91cb-5190e8d69169",
Contributor

Alys commented May 27, 2017

  • There's a bug that occurs if a user has the website open in two browser tabs or two different browsers - for example, at home and at work. The most troublesome aspects are highlighted in bold below.
    If the website is left open overnight on both browsers, the regular 6-hour reload will trigger cron in both.
    The first cron process runs just once, as you can see if there's a quest in progress - there's only one boss quest message at that point (we have protections in place to stop cron running twice).
    However the "Did you complete any of these?" modal appears on both browsers.
    The user will see it first at home in the morning and will tick the appropriate tasks then dismiss the modal.
    Habitica then processes all of the tasks from the yesterDailies array as intended (and a second quest progress message appears in party chat, as listed in one of the bugs above).
    The user then goes to work and sees the same "Did you complete any of these?" modal there too - there's no indication that the modal should no longer be used.
    The user could think that their actions at home weren't recorded, or could think that they must have forgotten to handle the modal at home.
    The user ticks the tasks again and the modal lets them do that. The user earns XP, etc from tasks that they'd already ticked at home, and that XP, etc remains in their account (it's not removed later after a sync).
    The user then dismisses the modal.
    Habitica then processes the tasks from the modal again.
    A third quest progress message appears in party chat, with damage to and from the boss being caused by the same tasks that resulted in the second quest progress message. The damage to the boss actually seems to be almost doubled for some reason, even when the same tasks are ticked on both modals. See the screenshot below -- all three quest progress messages are within a couple of minutes since I was doing this as a test but in the real scenario I described, the first message might be from say 2am when the browser reloads, the second might be from 6am when the user starts using Habitica at home, and the third from 9am at work.
    The tasks that were completed in the modal at work have remained completed in the user's tasks list - they are not uncompleted and ready to be used from scratch today.
    The user can uncomplete them manually, and then the user loses the XP, etc that they'd gained inappropriately, so that part is correct, but this will be confusing to many the users.

Similar behaviour will probably occur if the user has Habitica open in a browser somehere but first uses it in a mobile app, or if they use a third-party tool that

The ideal behaviour is that when the "Did you complete any of these?" modal appears in more than one tab or browser or device, it should only be actionable once. It should be impossible for the user to do anything on the second modal except close it.

image

"chat": [
    {
        "id": "c264c958-da79-4397-9f24-20eebac1edfc",
        "text": "`Canasta attacks Vice's Shade for 4.8 damage.` `Vice's Shade attacks party for 1.3 damage.`",
        "timestamp": 1495898346389,	// 	UTC: 2017-05-27 15:19:06
        "flagCount": 0
    },
    {
        "id": "a6f6f287-2c66-4069-a487-6607c2c7a3a9",
        "text": "`Canasta attacks Vice's Shade for 2.5 damage.` `Vice's Shade attacks party for 1.3 damage.`",
        "timestamp": 1495898339622,	// 	UTC: 2017-05-27 15:18:59
        "flagCount": 0
    },
    {
        "id": "197d79be-b64e-4e7e-82ec-8130090d0798",
        "text": "`Canasta attacks Vice's Shade for 20.7 damage.` `Vice's Shade attacks party for 7.9 damage.`",
        "timestamp": 1495898295840,	// 	UTC: 2017-05-27 15:18:15
        "flagCount": 0
    },
    {
        "id": "340b85e1-33ea-421e-aa8b-b353a3e42c1c",
        "text": "message before dual browser test",
        "timestamp": 1495898214878,	// 	UTC: 2017-05-27 15:16:54
        "flagCount": 0,
        "user": "Canasta"
    },
    {
        "id": "4c158b20-bb19-4727-91cb-5190e8d69169",
@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn May 31, 2017

Contributor

The bug you describe with the two browsers is an issue with the user being able to complete a task twice, correct? The yesterdailies is not processed. Therefore, this is already an issue with the production site because a user could have "two tabs" and mark a task completed twice.

I can fix that in this PR, but I am pretty sure that is a sync issue.

Contributor

TheHollidayInn commented May 31, 2017

The bug you describe with the two browsers is an issue with the user being able to complete a task twice, correct? The yesterdailies is not processed. Therefore, this is already an issue with the production site because a user could have "two tabs" and mark a task completed twice.

I can fix that in this PR, but I am pretty sure that is a sync issue.

@Alys

This comment has been minimized.

Show comment
Hide comment
@Alys

Alys May 31, 2017

Contributor

The bug you describe with the two browsers is an issue with the user being able to complete a task twice, correct?

Yes, from a technical point of view, it's much the same thing, but from a user's perspective, it's very different and would give a strong impression of being a bug. When you see the tasks in the yesterdailies modal, Habitica is saying to you that you are marking them complete for yesterday not for today, so when you dismiss the modal, it looks very wrong that the tasks are still marked complete in today's list.

The problem with the extra quest progress message is not on the production site.

Contributor

Alys commented May 31, 2017

The bug you describe with the two browsers is an issue with the user being able to complete a task twice, correct?

Yes, from a technical point of view, it's much the same thing, but from a user's perspective, it's very different and would give a strong impression of being a bug. When you see the tasks in the yesterdailies modal, Habitica is saying to you that you are marking them complete for yesterday not for today, so when you dismiss the modal, it looks very wrong that the tasks are still marked complete in today's list.

The problem with the extra quest progress message is not on the production site.

@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn May 31, 2017

Contributor

Yea, I definitely agree that this PR makes the bug
more apparent. I'll check on the quest progress as well.

Contributor

TheHollidayInn commented May 31, 2017

Yea, I definitely agree that this PR makes the bug
more apparent. I'll check on the quest progress as well.

@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn Jun 2, 2017

Contributor
  • Updated On task edit screen, "Mark this as a Yesterdaily" could be changed to something like "Prompt to check off if missed"
  • Updated I think that text would also benefit from an on-hover tip in the same way that "Task Alias" has a tip. We could use placeholder text for now until we work out what explanations are needed.
  • This is an issue with the debug controls I believe. Same thing happened during checkins test. I can cron and the pop up will come up. However, after that, if I hit the +days without refreshing for the second time, the popup will not show until refresh When I cron, the Record Yesterday's Activity modal usualy doesn't appear until I've reloaded the page.
  • We addressed this issue last run with allows users to change the group tasks and challenges tasks yesterdaily status. I think it is expected that a group or challenge leader can control this. And, they might report as a bug if they can't A Group Plan leader or manager is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a member claims the task, it should be a yesterdaily by default
  • Same as above A Group Plan leader or manager is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a member claims the task, it should be a yesterdaily by default
  • Updated, but I tested checking off all checklist items on a non-yesterdaily that was incomplete and I still took damage. The damage was less though. If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers. The correct behaviour is that when the Daily is not complete but some checklist items are ticked, then the user takes less damage. If all checklist items are complete, the user should take no damage from that Daily (even when the Daily itself is not marked complete).
  • Updated If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete, and then in the yesterdailies modal they mark the Daily itself as complete, the checklist items are not unticked (they should become unticked).
  • Updated If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, the Daily becomes grey and ticked. It should remain coloured and unticked but with the "approval requested" text on it.
  • This would be pretty hard, I believe. The cron runs separately from the again dailies. We would have to hold damage from cron and keep it synced with when a user final ages their dailies. I think that could lead to some buggy behavior. If a user has no yesterdalies, the message would never appear. Open to suggestions though. When the player is in a quest and has Dailies that they mark complete in the yesterdailies modal, two quest progress messages are produced in party chat instead of one (one message for the damage accumulated before the yesterdailies modal appears and one for the Dailies processed in the yesterdailies modal). Ideally a single message will be produced since all of that damage is for a single day.
  • Updated When the player marks Dailies complete in the yesterdailies modal, they can get drops even if they already met their drop cap for the previous day (ideally that wouldn't happen because the tasks they're marking as complete are for the previous day and so shouldn't allow them to exceed that day's drop cap). The drops they get count towards the new day's drop cap (also not the ideal behaviour since it reduces one of the motivational features that encourages them to complete new tasks on the new day).
  • Updated If the user marks a Daily as complete in the modal and then realises they ticked the wrong one, they'll try to untick it but they can't - the Daily stays ticked and grey. If they keey trying to untick it, they keep losing GP, XP, and MP for every click. The desired behaviour is that the first tick should complete it, the second tick should uncomplete it, a third tick should complete it again, etc.
Contributor

TheHollidayInn commented Jun 2, 2017

  • Updated On task edit screen, "Mark this as a Yesterdaily" could be changed to something like "Prompt to check off if missed"
  • Updated I think that text would also benefit from an on-hover tip in the same way that "Task Alias" has a tip. We could use placeholder text for now until we work out what explanations are needed.
  • This is an issue with the debug controls I believe. Same thing happened during checkins test. I can cron and the pop up will come up. However, after that, if I hit the +days without refreshing for the second time, the popup will not show until refresh When I cron, the Record Yesterday's Activity modal usualy doesn't appear until I've reloaded the page.
  • We addressed this issue last run with allows users to change the group tasks and challenges tasks yesterdaily status. I think it is expected that a group or challenge leader can control this. And, they might report as a bug if they can't A Group Plan leader or manager is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a member claims the task, it should be a yesterdaily by default
  • Same as above A Group Plan leader or manager is able to turn off the yesterdailies checkbox when editing a task. They should not be able to (when a member claims the task, it should be a yesterdaily by default
  • Updated, but I tested checking off all checklist items on a non-yesterdaily that was incomplete and I still took damage. The damage was less though. If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete in the yesterdailies modal but does not mark the Daily itself as complete, the checklist items are not used to reduce the damage that the user suffers. The correct behaviour is that when the Daily is not complete but some checklist items are ticked, then the user takes less damage. If all checklist items are complete, the user should take no damage from that Daily (even when the Daily itself is not marked complete).
  • Updated If an incomplete Daily has a checklist and the user marks some or all of the checklist items complete, and then in the yesterdailies modal they mark the Daily itself as complete, the checklist items are not unticked (they should become unticked).
  • Updated If a Group Plan yesterDaily that requires approval is marked complete from within the yesterdailies modal, the Daily becomes grey and ticked. It should remain coloured and unticked but with the "approval requested" text on it.
  • This would be pretty hard, I believe. The cron runs separately from the again dailies. We would have to hold damage from cron and keep it synced with when a user final ages their dailies. I think that could lead to some buggy behavior. If a user has no yesterdalies, the message would never appear. Open to suggestions though. When the player is in a quest and has Dailies that they mark complete in the yesterdailies modal, two quest progress messages are produced in party chat instead of one (one message for the damage accumulated before the yesterdailies modal appears and one for the Dailies processed in the yesterdailies modal). Ideally a single message will be produced since all of that damage is for a single day.
  • Updated When the player marks Dailies complete in the yesterdailies modal, they can get drops even if they already met their drop cap for the previous day (ideally that wouldn't happen because the tasks they're marking as complete are for the previous day and so shouldn't allow them to exceed that day's drop cap). The drops they get count towards the new day's drop cap (also not the ideal behaviour since it reduces one of the motivational features that encourages them to complete new tasks on the new day).
  • Updated If the user marks a Daily as complete in the modal and then realises they ticked the wrong one, they'll try to untick it but they can't - the Daily stays ticked and grey. If they keey trying to untick it, they keep losing GP, XP, and MP for every click. The desired behaviour is that the first tick should complete it, the second tick should uncomplete it, a third tick should complete it again, etc.
@phillipthelen

This comment has been minimized.

Show comment
Hide comment
@phillipthelen

phillipthelen Jun 7, 2017

Member

Documentation of a discussion:

The way yesterdailies and cron will be changed. Essentially the following changes will be made:

  • Cron is not checked/run on API calls automatically anymore.
  • Instead there will be a special API call /cron that runs it.
  • all dailies are handled the same on the server regardless if they are yesterdailies or not.
  • On the client side,
    • the client checks if there are any uncompleted yesterdailies,
    • displays those yesterdailies (if available),
    • lets the user score the yesterdailies
    • then runs the cron API call.

This change is made to later enable an offline mode for the mobile clients, so that they can score all tasks that were checked off when they were offline before cron is run.

Member

phillipthelen commented Jun 7, 2017

Documentation of a discussion:

The way yesterdailies and cron will be changed. Essentially the following changes will be made:

  • Cron is not checked/run on API calls automatically anymore.
  • Instead there will be a special API call /cron that runs it.
  • all dailies are handled the same on the server regardless if they are yesterdailies or not.
  • On the client side,
    • the client checks if there are any uncompleted yesterdailies,
    • displays those yesterdailies (if available),
    • lets the user score the yesterdailies
    • then runs the cron API call.

This change is made to later enable an offline mode for the mobile clients, so that they can score all tasks that were checked off when they were offline before cron is run.

@TheHollidayInn

This comment has been minimized.

Show comment
Hide comment
@TheHollidayInn

TheHollidayInn Jun 8, 2017

Contributor

Closing this to start a new PR.

Contributor

TheHollidayInn commented Jun 8, 2017

Closing this to start a new PR.

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