Group plans reorder tasks (#8358)

* Added move route for group tasks

* Added group task reorder to front end

* Added syncing with group task order

* Fixed linting issues

* Added missing exec and abstracted move code

* Added unit test for moveTask

test/api/v3/integration/tasks/groups/POST-tasks_move_taskId_to_position.test.js

@@ -0,0 +1,49 @@
+import {
+  generateUser,
+  generateGroup,
+} from '../../../../../helpers/api-v3-integration.helper';
+
+describe('POST group-tasks/:taskId/move/to/:position', () => {
+  let user, guild;
+
+  beforeEach(async () => {
+    user = await generateUser({balance: 1});
+    guild = await generateGroup(user, {type: 'guild'});
+  });
+
+  it('can move task to new position', async () => {
+    let tasks = await user.post(`/tasks/group/${guild._id}`, [
+      {type: 'habit', text: 'habit 1'},
+      {type: 'habit', text: 'habit 2'},
+      {type: 'daily', text: 'daily 1'},
+      {type: 'habit', text: 'habit 3'},
+      {type: 'habit', text: 'habit 4'},
+      {type: 'todo', text: 'todo 1'},
+      {type: 'habit', text: 'habit 5'},
+    ]);
+
+    let taskToMove = tasks[1];
+    expect(taskToMove.text).to.equal('habit 2');
+    let newOrder = await user.post(`/group-tasks/${tasks[1]._id}/move/to/3`);
+    expect(newOrder[3]).to.equal(taskToMove._id);
+    expect(newOrder.length).to.equal(5);
+  });
+
+  it('can push to bottom', async () => {
+    let tasks = await user.post(`/tasks/group/${guild._id}`, [
+      {type: 'habit', text: 'habit 1'},
+      {type: 'habit', text: 'habit 2'},
+      {type: 'daily', text: 'daily 1'},
+      {type: 'habit', text: 'habit 3'},
+      {type: 'habit', text: 'habit 4'},
+      {type: 'todo', text: 'todo 1'},
+      {type: 'habit', text: 'habit 5'},
+    ]);
+
+    let taskToMove = tasks[1];
+    expect(taskToMove.text).to.equal('habit 2');
+    let newOrder = await user.post(`/group-tasks/${tasks[1]._id}/move/to/-1`);
+    expect(newOrder[4]).to.equal(taskToMove._id);
+    expect(newOrder.length).to.equal(5);
+  });
+});

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

@@ -2,6 +2,7 @@ import {
   createTasks,
   getTasks,
   syncableAttrs,
+  moveTask,
 } from '../../../../../website/server/libs/taskManager';
 import i18n from '../../../../../website/common/script/i18n';
 import {
@@ -169,4 +170,12 @@ describe('taskManager', () => {
     expect(syncableTask.notes).to.not.exist;
     expect(syncableTask.updatedAt).to.not.exist;
   });
+
+  it('moves tasks to a specified position', async() => {
+    let order = ['task-id-1', 'task-id-2'];
+
+    moveTask(order, 'task-id-2', 0);
+
+    expect(order).to.eql(['task-id-2', 'task-id-1']);
+  });
 });

test/client-old/spec/services/taskServicesSpec.js

@@ -90,6 +90,14 @@ describe('Tasks Service', function() {
     $httpBackend.flush();
   });
 
+  it('calls group move task endpoint', function() {
+    var taskId = 1;
+    var position = 0;
+    $httpBackend.expectPOST('/api/v3/group-tasks/' + taskId + '/move/to/' + position).respond({});
+    tasks.moveGroupTask(taskId, position);
+    $httpBackend.flush();
+  });
+
   it('calls add check list item endpoint', function() {
     var taskId = 1;
     $httpBackend.expectPOST(apiV3Prefix + '/' + taskId + '/checklist').respond({});

website/client-old/js/app.js

@@ -198,10 +198,28 @@ window.habitrpg = angular.module('habitrpg',
               })
               .then(function (response) {
                 var tasks = response.data.data;
-                tasks.forEach(function (element, index, array) {
-                  if (!$scope.group[element.type + 's']) $scope.group[element.type + 's'] = [];
-                  $scope.group[element.type + 's'].unshift(element);
-                });
+
+                // @TODO: This task ordering logic should be astracted and user everywhere group or user tasks are loaded
+                var groupedTasks = _(tasks)
+                  .groupBy('type')
+                  .forEach(function (tasksOfType, type) {
+                    var order = $scope.group.tasksOrder[type + 's'];
+                    var orderedTasks = new Array(tasksOfType.length);
+                    var unorderedTasks = []; // what we want to add later
+
+                    tasksOfType.forEach(function (task, index) {
+                      var taskId = task._id;
+                      var i = order[index] === taskId ? index : order.indexOf(taskId);
+                      if (i === -1) {
+                        unorderedTasks.unshift(task); // unshift because we want to display new on top
+                      } else {
+                        orderedTasks[i] = task;
+                      }
+                    });
+
+                    // Remove empty values from the array and add any unordered task
+                    $scope.group[type + 's'] = _.compact(orderedTasks).concat(unorderedTasks);
+                  }).value();
 
                 $scope.group.approvals = [];
                 if (User.user._id === $scope.group.leader._id) {

website/client-old/js/directives/hrpg-sort-tasks.directive.js

@@ -6,10 +6,11 @@
     .directive('hrpgSortTasks', hrpgSortTasks);
 
   hrpgSortTasks.$inject = [
-    'User'
+    'User',
+    'Tasks',
   ];
 
-  function hrpgSortTasks(User) {
+  function hrpgSortTasks(User, Tasks) {
     return function($scope, element, attrs, ngModel) {
       $(element).sortable({
         axis: "y",
@@ -20,6 +21,13 @@
         stop: function (event, ui) {
           var task = angular.element(ui.item[0]).scope().task;
           var startIndex = ui.item.data('startIndex');
+
+          // Check if task is a group original task
+          if (task.group.id && !task.userId) {
+            Tasks.moveGroupTask(task._id, ui.item.index());
+            return;
+          }
+
           User.sortTask({
             params: { id: task._id, taskType: task.type },
             query: {

website/client-old/js/services/taskServices.js

@@ -158,6 +158,13 @@ angular.module('habitrpg')
       });
     };
 
+    function moveGroupTask (taskId, position) {
+      return $http({
+        method: 'POST',
+        url: '/api/v3/group-tasks/' + taskId + '/move/to/' + position,
+      });
+    };
+
     function addChecklistItem (taskId, checkListItem) {
       return $http({
         method: 'POST',
@@ -408,5 +415,6 @@ angular.module('habitrpg')
 
       getGroupApprovals: getGroupApprovals,
       approve: approve,
+      moveGroupTask: moveGroupTask,
     };
   }]);

website/server/controllers/api-v3/tasks.js

@@ -16,6 +16,7 @@ import {
 import {
   createTasks,
   getTasks,
+  moveTask,
 } from '../../libs/taskManager';
 import common from '../../../common';
 import Bluebird from 'bluebird';
@@ -460,22 +461,8 @@ api.moveTask = {
     if (!task) throw new NotFound(res.t('taskNotFound'));
     if (task.type === 'todo' && task.completed) throw new BadRequest(res.t('cantMoveCompletedTodo'));
     let order = user.tasksOrder[`${task.type}s`];
-    let currentIndex = order.indexOf(task._id);
 
-    // If for some reason the task isn't ordered (should never happen), push it in the new position
-    // if the task is moved to a non existing position
-    // or if the task is moved to position -1 (push to bottom)
-    // -> push task at end of list
-    if (!order[to] && to !== -1) {
-      order.push(task._id);
-    } else {
-      if (currentIndex !== -1) order.splice(currentIndex, 1);
-      if (to === -1) {
-        order.push(task._id);
-      } else {
-        order.splice(to, 0, task._id);
-      }
-    }
+    moveTask(order, task._id, to);
 
     await user.save();
     res.respond(200, order);

website/server/controllers/api-v3/tasks/groups.js

@@ -4,12 +4,14 @@ import * as Tasks from '../../../models/task';
 import { model as Group } from '../../../models/group';
 import { model as User } from '../../../models/user';
 import {
+  BadRequest,
   NotFound,
   NotAuthorized,
 } from '../../../libs/errors';
 import {
   createTasks,
   getTasks,
+  moveTask,
 } from '../../../libs/taskManager';
 
 let requiredGroupFields = '_id leader tasksOrder name';
@@ -80,6 +82,58 @@ api.getGroupTasks = {
   },
 };
 
+/**
+ * @api {post} /api/v3/group/:groupId/tasks/:taskId/move/to/:position Move a group task to a specified position
+ * @apiDescription Moves a group task to a specified position
+ * @apiVersion 3.0.0
+ * @apiName GroupMoveTask
+ * @apiGroup Task
+ *
+ * @apiParam {String} taskId The task _id
+ * @apiParam {Number} position Query parameter - Where to move the task (-1 means push to bottom). First position is 0
+ *
+ * @apiSuccess {Array} data The new tasks order (group.tasksOrder.{task.type}s)
+ */
+api.groupMoveTask = {
+  method: 'POST',
+  url: '/group-tasks/:taskId/move/to/:position',
+  middlewares: [authWithHeaders()],
+  async handler (req, res) {
+    req.checkParams('taskId', res.t('taskIdRequired')).notEmpty();
+    req.checkParams('position', res.t('positionRequired')).notEmpty().isNumeric();
+
+    let reqValidationErrors = req.validationErrors();
+    if (reqValidationErrors) throw reqValidationErrors;
+
+    let user = res.locals.user;
+
+    let taskId = req.params.taskId;
+    let task = await Tasks.Task.findOne({
+      _id: taskId,
+    }).exec();
+
+    let to = Number(req.params.position);
+
+    if (!task) {
+      throw new NotFound(res.t('taskNotFound'));
+    }
+
+    if (task.type === 'todo' && task.completed) throw new BadRequest(res.t('cantMoveCompletedTodo'));
+
+    let group = await Group.getGroup({user, groupId: task.group.id, fields: requiredGroupFields});
+    if (!group) throw new NotFound(res.t('groupNotFound'));
+
+    if (group.leader !== user._id) throw new NotAuthorized(res.t('onlyGroupLeaderCanEditTasks'));
+
+    let order = group.tasksOrder[`${task.type}s`];
+
+    moveTask(order, task._id, to);
+
+    await group.save();
+    res.respond(200, order);
+  },
+};
+
 /**
  * @api {post} /api/v3/tasks/:taskId/assign/:assignedUserId Assign a group task to a user
  * @apiDescription Assigns a user to a group task

website/server/libs/taskManager.js

@@ -188,3 +188,32 @@ export function syncableAttrs (task) {
   if (t.type !== 'reward') omitAttrs.push('value');
   return _.omit(t, omitAttrs);
 }
+
+/**
+ * Moves a task to a specified position.
+ *
+ * @param  order  The list of ordered tasks
+ * @param  taskId  The Task._id of the task to move
+ * @param  to A integer specifiying the index to move the task to
+ *
+ * @return Empty
+ */
+export function moveTask (order, taskId, to) {
+  let currentIndex = order.indexOf(taskId);
+
+  // If for some reason the task isn't ordered (should never happen), push it in the new position
+  // if the task is moved to a non existing position
+  // or if the task is moved to position -1 (push to bottom)
+  // -> push task at end of list
+  if (!order[to] && to !== -1) {
+    order.push(taskId);
+    return;
+  }
+
+  if (currentIndex !== -1) order.splice(currentIndex, 1);
+  if (to === -1) {
+    order.push(taskId);
+  } else {
+    order.splice(to, 0, taskId);
+  }
+}