From a559c1add8386be81ef27ef07767fb15426f3851 Mon Sep 17 00:00:00 2001 From: SabreCat Date: Fri, 3 Jun 2022 16:40:09 -0500 Subject: [PATCH] refactor(tasks): get rid of behind-the-scenes task cloning --- website/server/controllers/api-v3/tasks.js | 24 +----- website/server/models/group.js | 95 ---------------------- 2 files changed, 1 insertion(+), 118 deletions(-) diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index 99ecb122bd..b3facd60a2 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/website/server/controllers/api-v3/tasks.js @@ -632,7 +632,6 @@ api.updateTask = { verifyTaskModification(task, user, group, challenge, res); } - const oldCheckList = task.checklist; // we have to convert task to an object because otherwise things // don't get merged correctly. Bad for performances? const [updatedTaskObj] = common.ops.updateTask(task.toObject(), req); @@ -688,23 +687,11 @@ api.updateTask = { setNextDue(task, user); const savedTask = await task.save(); - if (group && task.group.id && task.group.assignedUsers) { - const updateCheckListItems = _.remove(sanitizedObj.checklist, checklist => { - const indexOld = _.findIndex(oldCheckList, check => check.id === checklist.id); - if (indexOld !== -1) return checklist.text !== oldCheckList[indexOld].text; - return false; // Only return changes. Adding and remove are handled differently - }); - - await group.updateTask(savedTask, { updateCheckListItems }); - } - res.respond(200, savedTask); if (challenge) { challenge.updateTask(savedTask); - } else if (group && task.group.id && task.group.assignedUsers) { - await group.updateTask(savedTask); - } else { + } else if (!group) { taskActivityWebhook.send(user, { type: 'updated', task: savedTask, @@ -954,9 +941,6 @@ api.addChecklistItem = { res.respond(200, savedTask); if (challenge) challenge.updateTask(savedTask); - if (group && task.group.id && task.group.assignedUsers.length > 0) { - await group.updateTask(savedTask, { newCheckListItem }); - } }, }; @@ -1061,9 +1045,6 @@ api.updateChecklistItem = { res.respond(200, savedTask); if (challenge) challenge.updateTask(savedTask); - if (group && task.group.id && task.group.assignedUsers.length > 0) { - await group.updateTask(savedTask); - } }, }; @@ -1123,9 +1104,6 @@ api.removeChecklistItem = { const savedTask = await task.save(); res.respond(200, savedTask); if (challenge) challenge.updateTask(savedTask); - if (group && task.group.id && task.group.assignedUsers.length > 0) { - await group.updateTask(savedTask, { removedCheckListItemId: req.params.itemId }); - } }, }; diff --git a/website/server/models/group.js b/website/server/models/group.js index b2e9edc6fd..5316e66c45 100644 --- a/website/server/models/group.js +++ b/website/server/models/group.js @@ -29,9 +29,6 @@ import { import baseModel from '../libs/baseModel'; import { sendTxn as sendTxnEmail } from '../libs/email'; // eslint-disable-line import/no-cycle import { sendNotification as sendPushNotification } from '../libs/pushNotifications'; // eslint-disable-line import/no-cycle -import { // eslint-disable-line import/no-cycle - syncableAttrs, -} from '../libs/tasks/utils'; import { schema as SubscriptionPlanSchema, } from './subscriptionPlan'; @@ -1444,58 +1441,6 @@ schema.methods.unlinkTags = function unlinkTags (user) { }); }; -/** - * Updates all linked tasks for a group task - * - * @param taskToSync The group task that will be synced - * @param options.newCheckListItem The new checklist item - * that needs to be synced to all assigned users - * @param options.removedCheckListItem The removed checklist item that - * needs to be removed from all assigned users - * - * @return The created tasks - */ -schema.methods.updateTask = async function updateTask (taskToSync, options = {}) { - const group = this; - - const updateCmd = { $set: {} }; - - const syncableAttributes = syncableAttrs(taskToSync); - for (const key of Object.keys(syncableAttributes)) { - updateCmd.$set[key] = syncableAttributes[key]; - } - - updateCmd.$set['group.assignedUsers'] = taskToSync.group.assignedUsers; - updateCmd.$set['group.managerNotes'] = taskToSync.group.managerNotes; - - const taskSchema = Tasks[taskToSync.type]; - - const updateQuery = { - userId: { $exists: true }, - 'group.id': group.id, - 'group.taskId': taskToSync._id, - }; - - if (options.newCheckListItem) { - const newCheckList = { completed: false }; - newCheckList.linkId = options.newCheckListItem.id; - newCheckList.text = options.newCheckListItem.text; - updateCmd.$push = { checklist: newCheckList }; - } - - if (options.removedCheckListItemId) { - updateCmd.$pull = { checklist: { linkId: { $in: [options.removedCheckListItemId] } } }; - } - - if (options.updateCheckListItems) { - updateCmd.$set.checklist = taskToSync.checklist; - } - - // Updating instead of loading and saving for performances, - // risks becoming a problem if we introduce more complexity in tasks - await taskSchema.update(updateQuery, updateCmd, { multi: true }).exec(); -}; - schema.methods.syncTask = async function groupSyncTask (taskToSync, users, assigningUser) { const group = this; const toSave = []; @@ -1532,46 +1477,6 @@ schema.methods.syncTask = async function groupSyncTask (taskToSync, users, assig group: group._id, }); } - - const findQuery = { - 'group.taskId': taskToSync._id, - userId: user._id, - 'group.id': group._id, - }; - - let matchingTask = await Tasks.Task.findOne(findQuery).exec(); // eslint-disable-line - - if (!matchingTask) { // If the task is new, create it - matchingTask = new Tasks[taskToSync.type](Tasks.Task.sanitize(syncableAttrs(taskToSync))); - matchingTask.group.id = taskToSync.group.id; - matchingTask.userId = user._id; - matchingTask.group.taskId = taskToSync._id; - user.tasksOrder[`${taskToSync.type}s`].unshift(matchingTask._id); - } else { - _.merge(matchingTask, syncableAttrs(taskToSync)); - // Make sure the task is in user.tasksOrder - const orderList = user.tasksOrder[`${taskToSync.type}s`]; - if (orderList.indexOf(matchingTask._id) === -1 && (matchingTask.type !== 'todo' || !matchingTask.completed)) orderList.push(matchingTask._id); - } - matchingTask.group.assignedUsers = taskToSync.group.assignedUsers; - matchingTask.group.managerNotes = taskToSync.group.managerNotes; - - // sync checklist - if (taskToSync.checklist) { - taskToSync.checklist.forEach(element => { - const newCheckList = { completed: false }; - newCheckList.linkId = element.id; - newCheckList.text = element.text; - matchingTask.checklist.push(newCheckList); - }); - } - - // don't override the notes, but provide it if not provided - if (!matchingTask.notes) matchingTask.notes = taskToSync.notes; - // add tag if missing - if (matchingTask.tags.indexOf(group._id) === -1) matchingTask.tags.push(group._id); - - toSave.push(matchingTask.save(), user.save()); } toSave.push(taskToSync.save()); return Promise.all(toSave);