HabitRPG / habitica

A habit tracker app which treats your goals like a Role Playing Game.
https://habitica.com
Other
11.8k stars 4.05k forks source link

"Task not found" when checking off to-do in dated view #6839

Closed rcmbal closed 8 years ago

rcmbal commented 8 years ago

Discovered

some consistent but strange behaviour when checking off a dated to-do while the to-do column is being sorted by date:

In addition to the usual messages (XP, MP, Gold), there's a blue message containing "Task not found", however the effects of checking off the task apply and the task shows up in the "done" section as well.

Tried this multiple times on both Safari (OS X 10.11.3 with version 9.0.3) as well as Chromium on Debian Jessie.

There were no errors in the JS console asides from:

InvalidAccessError openbutton:1:839 aSrapid-3.33.js:1:10524 aUrapid-3.33.js:1:13300 aOrapid-3.33.js:1:14295 aBrapid-3.33.js:1:12791 sendRapidNoDelayrapid-3.33.js:1:15304 airapid-3.33.js:1:30878 Rapidrapid-3.33.js:1:31006 (anonymous function)button:70 (anonymous function)button:72

User-ID: d70bcf05-bc28-45d1-99e7-dca99cb0ec0b

I hope that this is not a duplicate of something I've missed.

ltorroba commented 8 years ago

Confirmed on Chrome @ Mac OS X:

Although in my tests no errors come up in the console whatsoever - perhaps the error you are getting is related to something else?

For me:

I will be taking a look at this later on today.

rcmbal commented 8 years ago

I think that the error from the console is not related, as it seems to point to something in the header.

What I just noticed now is that the same message appears when moving tasks while the dated view is selected, but maybe that could be a different problem.

Alys commented 8 years ago

You can ignore the "Task not found" message. It occurs when a task is moved from one position in your task list to another if the new neighbour task in the new position can't be found - it's just an indication that your action to change the sort order of your To-Dos might not have occurred correctly.

It can occur when ticking off a To-Do because completed To-Dos are automatically moved to the bottom of your To-Do list.

You're probably finding that you see the message whenever you have a To-Do filter applied (e.g., you're using a tag or using the "Dated" To-Do tab), but not at all or hardly even when there's no filter applied (e.g., no tag and the "Active" To-Do tab) -- this is because having a filter in place does make it harder for Habitica to accurately sort To-Dos.

We should remove that notification. It's not really helpful. In the cases where it does accurately warn you that sorting has failed, you'd notice that anyway soon enough when you saw your tasks in the original positions. I'll leave this here for a couple of days to see if there are any objections to that, and then if not, mark it as a change we want made.

doyler commented 8 years ago

@Alys - I'll gladly take a look into this, but just wanted a little clarification. I was able to reproduce this issue entirely, but do we want to remove the notification entirely? If so, I should have this done fairly soon!

Alys commented 8 years ago

Yes, no one complained to that suggestion above, so let's remove it entirely.

rbreu commented 8 years ago

Is there any progress on this? Otherwise I'd like to look into this.

rbreu commented 8 years ago

I've digged into it and found that sorting generally doesn't work in Dated mode: If you use the 'to top'/'to bottom' icons or try to drag and drop, you get the same "task not found" error. The sort function works on the todo-list without taking the dated-order into account. And when you check off a todo, the checked off task gets pushed to the bottom of the list using the same sort function so that you can view your completed todos in the order you completed them – or at least that is the plan, but the failing makes the checked off todo not appear in the right position.

Offering sorting functions on the dated view doesn't really make sense, does it? And for checking off the task, one should maybe manually push the task to the bottom without using the sort function?

Alys commented 8 years ago

@rbreu Did you see my message above? It describes the problem and says that the notification can be removed.

I'm not sure whether we should remove the sort buttons in the dated view or allow them to work but with an effect that's only noticeable when you move back to the active view. I would quite like to be able to hit "push to top" in dated view for all the Dailies due today so that I can then work on them in the active view along with all undated tasks that I've pushed to top. But that might be just me. :) Regardless, that needs further discussion.

You can remove the "Task not found" notification now if you wish.

rbreu commented 8 years ago

I wouldn't feel comfortable removing the message as it originates in the overall sort function, which is used in a lot of different places (on server side as well), and the message itself is correct in stating that the sort function is being used in the wrong way.

If we don't care that the checked-off task ends up in the wrong position in the "Done" tab, I'd rather skip the attempt to sort beforehand if we're operating in dated mode, which has the same effect to the user as removing the message. Or preferably fix the sorting attempt so that the checked off task does end up in the right position in the Done column. (Though it should get sorted correctly on server side and show up in the proper order on page reload, so maybe not a top priority.) But I'd rather wait until there's a decision on what to do with the sorting buttons and fix it all in one go and consistently.

While pushing to top/bottom at least has a definite expected outcome (even if maybe confusing), I can't see how drag & drop would work, seeing as the drop position is pretty meaningless if you don't see the "actual" order.

crookedneighbor commented 8 years ago

I'm inclined to agree with @rbeu. Solving this by just removing the message is a code smell and will lead to unintended consequences.

Alys commented 8 years ago

No worries. Now that there's objections to the suggestion, I'll put this back to suggestion-discussion.

rbreu commented 8 years ago

Just to be clear, the issue arises because the sorting/filtering is done on-the-fly in the template, so that the client controller has no direct access to the displayed order when trying to reorder tasks, but it's given the indexes of the displayed list to work with.

The searchTasks function already is a bit unwieldy because it tries to work around some of those issues (I think that's why though I haven't completely grasped it yet). Seeing that there are a couple of Trello suggestions regarding new sort and filter options, it's probably worth investigating, though I'm not sure what the smartest solution is here.

paglias commented 8 years ago

I think this can be closed now, reopen if it occurs again