Task order fix (#8928)

test/api/v3/unit/libs/cron.test.js

@@ -365,6 +365,72 @@ describe('cron', () => {
 
       expect(user.history.todos).to.be.lengthOf(1);
     });
+
+    it('should remove completed todos from users taskOrder list', () => {
+      tasksByType.todos = [];
+      user.tasksOrder.todos = [];
+      let todo = {
+        text: 'test todo',
+        type: 'todo',
+        value: 0,
+      };
+
+      let task = new Tasks.todo(Tasks.Task.sanitize(todo)); // eslint-disable-line new-cap
+      tasksByType.todos.push(task);
+      task = new Tasks.todo(Tasks.Task.sanitize(todo)); // eslint-disable-line new-cap
+      tasksByType.todos.push(task);
+      tasksByType.todos[0].completed = true;
+
+      user.tasksOrder.todos = tasksByType.todos.map(taskTodo => {
+        return taskTodo._id;
+      });
+      // Since ideally tasksByType should not contain completed todos, fake ids should be filtered too
+      user.tasksOrder.todos.push('00000000-0000-0000-0000-000000000000');
+
+      expect(tasksByType.todos).to.be.lengthOf(2);
+      expect(user.tasksOrder.todos).to.be.lengthOf(3);
+
+      cron({user, tasksByType, daysMissed, analytics});
+
+      // user.tasksOrder.todos should be filtered while tasks by type remains unchanged
+      expect(tasksByType.todos).to.be.lengthOf(2);
+      expect(user.tasksOrder.todos).to.be.lengthOf(1);
+    });
+
+    it('should preserve todos order in task list', () => {
+      tasksByType.todos = [];
+      user.tasksOrder.todos = [];
+      let todo = {
+        text: 'test todo',
+        type: 'todo',
+        value: 0,
+      };
+
+      let task = new Tasks.todo(Tasks.Task.sanitize(todo)); // eslint-disable-line new-cap
+      tasksByType.todos.push(task);
+      task = new Tasks.todo(Tasks.Task.sanitize(todo)); // eslint-disable-line new-cap
+      tasksByType.todos.push(task);
+      task = new Tasks.todo(Tasks.Task.sanitize(todo)); // eslint-disable-line new-cap
+      tasksByType.todos.push(task);
+
+      // Set up user.tasksOrder list in a specific order
+      user.tasksOrder.todos = tasksByType.todos.map(todoTask => {
+        return todoTask._id;
+      }).reverse();
+      let original = user.tasksOrder.todos; // Preserve the original order
+
+      cron({user, tasksByType, daysMissed, analytics});
+
+      let listsAreEqual = true;
+      user.tasksOrder.todos.forEach((taskId, index) => {
+        if (original[index]._id !== taskId) {
+          listsAreEqual = false;
+        }
+      });
+
+      expect(listsAreEqual);
+      expect(user.tasksOrder.todos).to.be.lengthOf(original.length);
+    });
   });
 
   describe('dailys', () => {

website/server/libs/cron.js

@@ -403,6 +403,13 @@ export function cron (options = {}) {
 
   user.history.exp.push({date: now, value: expTally});
 
+  // Remove any remaining completed todos from the list of active todos
+  user.tasksOrder.todos = user.tasksOrder.todos.filter(taskOrderId => {
+    return _.some(tasksByType.todos, taskType => {
+      return taskType._id === taskOrderId && taskType.completed === false;
+    });
+  });
+
   // preen user history so that it doesn't become a performance problem
   // also for subscribed users but differently
   // TODO also do while resting in the inn. Note that later we'll be allowing the value/color of tasks to change while sleeping (https://github.com/HabitRPG/habitica/issues/5232), so the code in performSleepTasks() might be best merged back into here for that. Perhaps wait until then to do preen history for sleeping users.