Habitica: everyX values for Dailies should not be allowed to be non-integers

Created on 3 Jun 2017  路  6Comments  路  Source: HabitRPG/habitica

The website API accepts non-integer values for everyX for a Daily, such as 0.5. This causes errors such as the one below.

Suggested fixes:

  • The API and/or task schema must prohibit anything other than an integer (including 0, which is currently allowed and should remain allowed until/if we implement a different way of making a Daily never due). It should also ensure that there IS a value for everyX - null is not permitted.
  • _Already fixed:_ The website should not allow you to type anything other than digits (i.e., the . character and all other non-digit characters should not be able to be entered). I recommend still allowing people to type numbers rather than select from a drop-down because drop-downs are annoying for anything more than a few numbers.

The Android app doesn't have this bug because you can't manually type the everyX value. I suspect but do not know that the iOS is the same and therefore also wouldn't need a fix, but someone should check that to be thorough.

Example of a task affected by this bug (I've fixed this task and I'm looking for all others affected to fix them too):

{
    "_id": "f489a688-01e7-43ae-b11e-fe8df7f6ee35",
   [snip irrelevant data]
    "userId": "b14c2d02-563a-4ba4-aa9d-efd20c183eb9",
    "updatedAt": "2017-05-24T07:25:31.105Z",
    "completed": false,
    "weeksOfMonth": [],
    "daysOfMonth": [
        1
    ],
    "streak": 0,
    "repeat": { "su": true, "s": true, "f": true, "th": true, "w": true, "t": true, "m": true },
    "startDate": "2017-05-01T04:00:00.000Z",
    "everyX": 0.5,
    "frequency": "daily",
    "isDue": true
}

Resulting error:


2017-06-01T04:06:54.695Z - error: Error: Intervals must be greater than zero
[snip]
170601_212551/i-00fcee367bbd7b16a/var/log/nodejs/rotated/nodejs.log-1496307661:    at processImmediate [as _immediateCallback] (timers.js:582:5) method=POST, originalUrl=/api/v3/tasks/236a33c9-0f04-4888-9ba1-197c4bdf1fb6/score/up?userV=367, host=habitica.com, content-length=2, accept=application/json, text/plain, */*, accept-encoding=gzip, deflate, br, accept-language=en-US,en;q=0.8, content-type=application/json;charset=UTF-8, dnt=1, origin=https://habitica.com, referer=https://habitica.com/, x-api-user=b14c2d02-563a-4ba4-aa9d-efd20c183eb9, x-client=habitica-web, x-forwarded-port=443, x-forwarded-proto=https, , httpCode=500, isHandledError=false, 
help wanted minor Task Page

All 6 comments

_Ugh._

{ "_id" : "0e0ffa12-3480-4fe5-8bcc-002348c2e138", "everyX" : null }
{ "_id" : "ff4b4c96-c98f-4155-9526-5d702a457d51", "everyX" : null }
{ "_id" : "d3fa5956-5bbe-42c3-b09c-9dbbc690df28", "everyX" : null }
{ "_id" : "0f79c0fe-c6ef-4e6b-9cbd-6efcec682228", "everyX" : null }
{ "_id" : "d23a4428-51f4-411e-b612-9ff230566991", "everyX" : null }
{ "_id" : "61c50e9a-2756-416a-a4a9-7305cefb86fc", "everyX" : -27009 }
{ "_id" : "c942ca7e-1791-4d3d-b446-8d11cfacf89d", "everyX" : -25963 }
{ "_id" : "f28f623f-0fdb-46f8-b1c2-db02972208e4", "everyX" : -10617 }
{ "_id" : "3aae6d6d-2bdb-4cab-af3c-aa8735ee3125", "everyX" : -7937 }
{ "_id" : "d23c0094-f8ff-4f64-bac9-b8c48aeee6d3", "everyX" : -7616 }
{ "_id" : "fa2d5e44-6c3b-4dba-8bd8-81f5699daee1", "everyX" : -7169 }
{ "_id" : "ba549f7e-54cd-4786-9735-30ddd9843243", "everyX" : -7169 }
{ "_id" : "1c44d0c3-9533-47de-983e-b4068a7420f7", "everyX" : -6145 }
{ "_id" : "94e367f4-808c-41e5-8454-88fae8fac461", "everyX" : -5888 }
{ "_id" : "8aea08ea-7fdb-42d3-8e4d-28bbfdf95d4f", "everyX" : -1 }
{ "_id" : "868d575a-550d-4fe2-8914-f06119cd00a2", "everyX" : -1 }
{ "_id" : "7a77994f-b9cc-4705-a335-ed81673570c9", "everyX" : -1 }
{ "_id" : "0481666b-bb24-4cdd-9e4d-0e037e5356d4", "everyX" : 99999 }
{ "_id" : "1c97d865-6c42-4e44-8d03-d7b914146a5e", "everyX" : 99999 }
{ "_id" : "ac4fc9d1-c144-4d39-89a8-c5f37d781098", "everyX" : 99999 }
{ "_id" : "c290f6e3-35cc-43b7-bf91-c1da209c70d1", "everyX" : 99999 }
{ "_id" : "5b44ded4-cfbe-4c38-a79d-cdf38bbd6b73", "everyX" : 99999 }
{ "_id" : "6ccdf777-f31c-493c-9214-5620b66f50f9", "everyX" : 99999 }
{ "_id" : "fc8a08cf-7d8b-41c3-a452-3a5683f89314", "everyX" : 99999 }
{ "_id" : "fc2dcbd4-9eab-4e14-b0da-1c61e9f2af6f", "everyX" : 99999 }
{ "_id" : "168b837e-f379-46a4-98b5-4aa3463fa61a", "everyX" : 600000 }
{ "_id" : "4d7f018f-a0fa-4b71-ba7f-a255c2554e6d", "everyX" : 999998 }
{ "_id" : "61605ed6-4ff9-4a40-822c-a51f2c1b9f70", "everyX" : 1000000 }
{ "_id" : "283c84ec-53b9-4c77-bc9a-65aa1922e027", "everyX" : 100000000 }
{ "_id" : "5cccf062-b1c9-41f6-84c2-2be01f1903aa", "everyX" : 1000000000 }
{ "_id" : "1b0dc79f-74ac-4ef8-a4d3-f1e02feabb1b", "everyX" : 40000000007 }
{ "_id" : "ff6d02cf-b5a9-48d6-8456-48bc6af3e38d", "everyX" : 999999999999999 }
{ "_id" : "8b2288e6-e9b3-4209-b3d3-677cbc568d09", "everyX" : 10000000000000000000 }
{ "_id" : "9a1b882f-5a75-44dc-940c-533cdfdb278d", "everyX" : 100000000000000000000 }
{ "_id" : "0108bde7-9adf-4c92-bc62-39eba2907833", "everyX" : 100000000000000000000 }
{ "_id" : "982c8132-0cb4-47d1-99aa-02605a0edcfa", "everyX" : 5e+21 }
{ "_id" : "98c27f57-bb05-48d7-9284-85584b7e2a48", "everyX" : 1e+22 }
{ "_id" : "32d90227-399d-45e2-80f7-a083aaccba8f", "everyX" : 1e+23 }
{ "_id" : "17af883b-3e29-4092-a517-4ef51dc7b273", "everyX" : 1e+30 }
{ "_id" : "028f92fe-3482-43db-b67e-59cb9f26fb17", "everyX" : 1e+32 }
{ "_id" : "e0b183d1-c764-4321-b67a-f96e26b57737", "everyX" : 1e+33 }

I'm changing those to 0 since that's probably more or less the users' intention.

I've updated the top post to state that null must not be allowed (i.e., the everyX value has to exist; while still being allowed to be zero).

Wow. This is being fixed in the performance PR as well.

Is this still in progress, or has it been fixed with the new client?

The website no longer allows non-integer values to be entered but I'm guessing the API does so that would still need to be fixed. If anyone looks at this and discovers that the API has already been changed, please comment here and we'll close this.

Labels updated.

@Alys createTask only validates that it is a number, not that it is an int.
task.js line 226
everyX: {type: Number, default: 1}, // e.g. once every X weeks
should be fixed just by changing it to
everyX: {type: Number, default: 1, validate: [ (val) => val % 1 === 0, 'Valid priority values are ints', ]},
There might be a cleaner way to accomplish that. Idk what effect this will have on existing non-int values in the database. I am willing to work on this task.

Thanks @TNychka ! It's yours.

Don't worry about existing non-int values in the database - admins will remove those before your PR is merged. If you could remind us of that in your PR, we'd be grateful. :)

Was this page helpful?
0 / 5 - 0 ratings