diff --git a/app/imports/api/engine/computation/computeComputation/tests/computeEffects.testFn.js b/app/imports/api/engine/computation/computeComputation/tests/computeEffects.testFn.js index 83ca46b3..733ede0d 100644 --- a/app/imports/api/engine/computation/computeComputation/tests/computeEffects.testFn.js +++ b/app/imports/api/engine/computation/computeComputation/tests/computeEffects.testFn.js @@ -2,6 +2,7 @@ import { buildComputationFromProps } from '/imports/api/engine/computation/build import { assert } from 'chai'; import computeCreatureComputation from '../../computeCreatureComputation'; import clean from '../../utility/cleanProp.testFn'; +import { propsFromForest } from '/imports/api/properties/tests/propTestBuilder.testFn'; export default function () { const computation = buildComputationFromProps(testProperties); diff --git a/app/imports/api/engine/computation/computeComputation/tests/index.js b/app/imports/api/engine/computation/computeComputation/tests/index.js index 32b8dda8..0faa0989 100644 --- a/app/imports/api/engine/computation/computeComputation/tests/index.js +++ b/app/imports/api/engine/computation/computeComputation/tests/index.js @@ -1,5 +1,6 @@ import computeAction from './computeAction.testFn'; import computeAttribute from './computeAttribute.testFn'; +import computeCalculations from './computeCalculations.testFn'; import computeClasses from './computeClasses.testFn'; import computeConstants from './computeConstants.testFn'; import computeInventory from './computeInventory.testFn'; @@ -7,6 +8,7 @@ import computeDamageMultipliers from './computeDamageMultipliers.testFn'; import computeEffects from './computeEffects.testFn'; import computeSkills from './computeSkills.testFn'; import computeProficiencies from './computeProficiencies.testFn'; +import computePointBuys from './computePointBuys.testFn'; export default [{ text: 'Computes actions', diff --git a/app/imports/api/parenting/organizeMethods.js b/app/imports/api/parenting/organizeMethods.js index 66d5a98a..848fc9c9 100644 --- a/app/imports/api/parenting/organizeMethods.js +++ b/app/imports/api/parenting/organizeMethods.js @@ -2,18 +2,72 @@ import SimpleSchema from 'simpl-schema'; import { ValidatedMethod } from 'meteor/mdg:validated-method'; import { RateLimiterMixin } from 'ddp-rate-limiter-mixin'; import { RefSchema } from '/imports/api/parenting/ChildSchema'; -import { assertDocEditPermission } from '/imports/api/sharing/sharingPermissions.js'; +import { assertDocEditPermission, assertEditPermission } from '/imports/api/sharing/sharingPermissions.js'; +import { compact } from 'lodash'; import Creatures from '/imports/api/creature/creatures/Creatures.js'; -import { changeParent, fetchDocByRefAsync, getCollectionByName, rebuildNestedSets } from '/imports/api/parenting/parentingFunctions'; +import { fetchDocByRefAsync, getCollectionByName, moveDocBetweenRoots, moveDocWithinRoot } from '/imports/api/parenting/parentingFunctions'; -const organizeDoc = new ValidatedMethod({ - name: 'organize.organizeDoc', +const moveBetweenRoots = new ValidatedMethod({ + name: 'organize.moveDocBetweenRoots', validate: new SimpleSchema({ docRef: RefSchema, - parentRef: RefSchema, - order: { - type: Number, - // Should end in 0.5 to place it reliably between two existing documents + newPosition: { + type: Number, // Must end in .5 + }, + newRootRef: RefSchema, + skipRecompute: { + type: Boolean, + optional: true, + }, + skipClient: { + type: Boolean, + optional: true, + }, + }).validator(), + mixins: [RateLimiterMixin], + rateLimit: { + numRequests: 5, + timeInterval: 5000, + }, + async run({ docRef, newPosition, newRootRef, skipRecompute, skipClient }) { + if (skipClient && this.isSimulation) { + return; + } + let doc = await fetchDocByRefAsync(docRef); + let collection = getCollectionByName(docRef.collection); + // The user must be able to edit both the doc and its parent to move it + // successfully + assertDocEditPermission(doc, this.userId); + const newRoot = await fetchDocByRefAsync(newRootRef); + assertEditPermission(newRoot, this.userId); + + + // Move the doc + await moveDocBetweenRoots(doc, collection, newRootRef, newPosition); + + // Figure out which creatures need to be recalculated after this move + const creatureIdsToRecalculate = compact([ + getCreatureAncestorId(doc), + getCreatureAncestorId(newRoot), + ]); + + // Mark the creatures for recompute + if (!skipRecompute && creatureIdsToRecalculate.length) { + Creatures.updateAsync({ + _id: { $in: creatureIdsToRecalculate }, + }, { + $set: { dirty: true }, + }); + } + }, +}); + +const moveWithinRoot = new ValidatedMethod({ + name: 'organize.moveDocWithinRoot', + validate: new SimpleSchema({ + docRef: RefSchema, + newPosition: { + type: Number, // Must end in .5 }, skipRecompute: { type: Boolean, @@ -29,76 +83,26 @@ const organizeDoc = new ValidatedMethod({ numRequests: 5, timeInterval: 5000, }, - async run({ docRef, parentRef, order, skipRecompute, skipClient }) { + async run({ docRef, newPosition, skipRecompute, skipClient }) { if (skipClient && this.isSimulation) { return; } let doc = await fetchDocByRefAsync(docRef); let collection = getCollectionByName(docRef.collection); - // The user must be able to edit both the doc and its parent to move it - // successfully + + // The user must be able to edit the doc assertDocEditPermission(doc, this.userId); - let parent; - parent = await fetchDocByRefAsync(parentRef); - assertDocEditPermission(parent, this.userId); - // Moving the doc to the root level means changing its parent to undefined - if (doc.root.id === parent._id) { - parent = null; - } - // Change the doc's parent - await changeParent(doc, parent, collection, order); + // Move the doc + await moveDocWithinRoot(doc, collection, newPosition); + + // Figure out which creature needs to be recalculated after this move + const creatureIdToRecalculate = getCreatureAncestorId(doc); - // Figure out which creatures need to be recalculated after this move - const docCreature = getCreatureAncestorId(doc); - const parentCreature = getCreatureAncestorId(parent); // Mark the creatures for recompute - if (!skipRecompute) { - if (docCreature) { - Creatures.updateAsync({ - _id: docCreature, - }, { - $set: { dirty: true }, - }); - } - if (parentCreature && parentCreature !== docCreature) { - Creatures.updateAsync({ - _id: parentCreature, - }, { - $set: { dirty: true }, - }); - } - } - }, -}); - -const reorderDoc = new ValidatedMethod({ - name: 'organize.reorderDoc', - validate: new SimpleSchema({ - docRef: RefSchema, - order: { - type: Number, - // Should end in 0.5 to place it reliably between two existing documents - }, - }).validator(), - mixins: [RateLimiterMixin], - rateLimit: { - numRequests: 5, - timeInterval: 5000, - }, - async run({ docRef, order }) { - const doc = await fetchDocByRefAsync(docRef); - assertDocEditPermission(doc, this.userId); - - let collection = getCollectionByName(docRef.collection); - console.log('setting doc left to ', order); - await collection.updateAsync(doc._id, { $set: { left: order } }); - - // Recompute the affected creatures - const creatureId = getCreatureAncestorId(doc); - if (creatureId) { - return Creatures.updateAsync({ - _id: creatureId + if (!skipRecompute && creatureIdToRecalculate) { + Creatures.updateAsync({ + _id: creatureIdToRecalculate, }, { $set: { dirty: true }, }); @@ -115,4 +119,4 @@ function getCreatureAncestorId(doc) { } } -export { organizeDoc, reorderDoc }; +export { moveBetweenRoots, moveWithinRoot }; diff --git a/app/imports/api/parenting/parentingFunctions.test.ts b/app/imports/api/parenting/parentingFunctions.test.ts index f9900200..ab42aaad 100644 --- a/app/imports/api/parenting/parentingFunctions.test.ts +++ b/app/imports/api/parenting/parentingFunctions.test.ts @@ -1,9 +1,12 @@ -import { docsToForest, calculateNestedSetOperations, getFilter } from '/imports/api/parenting/parentingFunctions' +import '/imports/api/simpleSchemaConfig'; +import { docsToForest, calculateNestedSetOperations, getFilter, moveDocWithinRoot } from '/imports/api/parenting/parentingFunctions' import { TreeDoc } from '/imports/api/parenting/ChildSchema'; import { assert } from 'chai'; function doc(_id, left, right, parentId?): TreeDoc { - return { _id, root: { id: 'root', collection: 'col' }, left, right, parentId }; + const doc = { _id, root: { id: 'root', collection: 'col' }, left, right, parentId }; + if (!parentId) delete doc.parentId; + return doc; } function op(_id, left, right) { @@ -184,3 +187,108 @@ describe('Document tree filters can fetch other documents based on their positio assert.sameMembers(parentIds['dbm'], ['Databases']); }); }); + +describe('Document can be moved without breaking the tree', function () { + /** + * Test the following structure + * + * 1 Books 12 13 Videos 24 + * ┃ ┃ + * 2 Programming 11 14 Cooking 23 + * ┏━━━━━━━━┻━━━━━━━━━┓ ┏━━━━━━━━┻━━━━━━━━━┓ + * 3 Languages 4 5 Databases 10 15 Meat 16 17 Vegetarian 22 + * ┏━━━━━━━┻━━━━━━━┓ ┏━━━━━━━┻━━━━━━━┓ + * 6 MongoDB 7 8 dbm 9 18 Pasta 19 20 Mains 21 + */ + const treeCollection: Mongo.Collection = new Mongo.Collection('treeDocsMove'); + beforeEach(function () { + treeCollection.remove({}); + [ + doc('Books', 1, 12, undefined), + doc('Programming', 2, 11, 'Books'), + doc('Languages', 3, 4, 'Programming'), + doc('Databases', 5, 10, 'Programming'), + doc('MongoDB', 6, 7, 'Databases'), + doc('dbm', 8, 9, 'Databases'), + doc('Videos', 13, 24, undefined), + doc('Cooking', 14, 23, 'Videos'), + doc('Meat', 15, 16, 'Cooking'), + doc('Vegetarian', 17, 22, 'Cooking'), + doc('Pasta', 18, 19, 'Vegetarian'), + doc('Mains', 20, 21, 'Vegetarian'), + ].map(doc => { + return treeCollection.insert(doc); + }); + }); + it('can move a document within its parent', async function () { + const languagesDoc = await treeCollection.findOneAsync({ _id: 'Languages' }); + if (!languagesDoc) throw new Error('Languages doc not found'); + await moveDocWithinRoot(languagesDoc, treeCollection, 10.5); + /** + * Expected resulting structure + * 1 Books 12 13 Videos 24 + * ┃ ┃ + * 2 Programming 11 14 Cooking 23 + * ┏━━━━━━━━┻━━━━━━━━━┓ ┏━━━━━━━━┻━━━━━━━━━┓ + * 3 Databases 8 9 Languages 10 15 Meat 16 17 Vegetarian 22 + * ┏━━━━━━━┻━━━━━━━┓ ┏━━━━━━━┻━━━━━━━┓ + * 4 MongoDB 5 6 dbm 7 18 Pasta 19 20 Mains 21 + */ + const docs = await treeCollection.find({}, { sort: { left: 1 } }).fetchAsync(); + assert.deepEqual(docs, [ + doc('Books', 1, 12, undefined), + doc('Programming', 2, 11, 'Books'), + doc('Databases', 3, 8, 'Programming'), + doc('MongoDB', 4, 5, 'Databases'), + doc('dbm', 6, 7, 'Databases'), + doc('Languages', 9, 10, 'Programming'), + doc('Videos', 13, 24, undefined), + doc('Cooking', 14, 23, 'Videos'), + doc('Meat', 15, 16, 'Cooking'), + doc('Vegetarian', 17, 22, 'Cooking'), + doc('Pasta', 18, 19, 'Vegetarian'), + doc('Mains', 20, 21, 'Vegetarian'), + ]); + }); + it('can move a document to a whole new parent', async function () { + const videos = await treeCollection.findOneAsync({ _id: 'Videos' }); + if (!videos) throw new Error('Videos doc not found'); + await moveDocWithinRoot(videos, treeCollection, 6.5); + /** + * Expected resulting structure + * 1 Books 24 + * ┃ + * 2 Programming 23 + * ┏━━━━━━━━┻━━━━━━━━━┓ + * 3 Languages 4 5 Databases 22 + * ┏━━━━━━━┻━━━━━━━┓ + * 6 MongoDB 19 20 dbm 21 + * ┃ + * 7 Videos 18 + * ┃ + * 8 Cooking 17 + * ┏━━━━━━━━┻━━━━━━━━━┓ + * 9 Meat 10 11 Vegetarian 16 + * ┏━━━━━━━┻━━━━━━━┓ + * 12 Pasta 13 14 Mains 15 + */ + const docs = await treeCollection.find({}, { sort: { left: 1 } }).fetchAsync(); + assert.deepEqual(docs, [ + doc('Books', 1, 24, undefined), + doc('Programming', 2, 23, 'Books'), + doc('Languages', 3, 4, 'Programming'), + doc('Databases', 5, 22, 'Programming'), + doc('MongoDB', 6, 19, 'Databases'), + doc('Videos', 7, 18, 'MongoDB'), + doc('Cooking', 8, 17, 'Videos'), + doc('Meat', 9, 10, 'Cooking'), + doc('Vegetarian', 11, 16, 'Cooking'), + doc('Pasta', 12, 13, 'Vegetarian'), + doc('Mains', 14, 15, 'Vegetarian'), + doc('dbm', 20, 21, 'Databases'), + ]); + }); +}); + + +// TODO test moving between roots diff --git a/app/imports/api/parenting/parentingFunctions.ts b/app/imports/api/parenting/parentingFunctions.ts index 330b4f88..4e5fcab6 100644 --- a/app/imports/api/parenting/parentingFunctions.ts +++ b/app/imports/api/parenting/parentingFunctions.ts @@ -341,9 +341,176 @@ export function renewDocIds({ docArray, collectionMap = {}, idMap = {} }) { }); } +/** + * Moves a document within its root. The destination must be halfway between two positions (n.5) + * @param doc + * @param collection + */ +export async function moveDocWithinRoot(doc: TreeDoc, collection: Mongo.Collection, newPosition: number) { + let move: number; + let includedRange; + // Ensure the destination is at a midway point + if (newPosition % 1 !== 0.5) { + throw new Meteor.Error('invalid-move', 'Destination must be halfway between two positions (n.5)'); + } + + // Get the distance to move and the range between the current document and the destination + const docSize = doc.right - doc.left + 1; + let shiftDistance; + if (newPosition < doc.left) { + move = newPosition - doc.left + 0.5; + includedRange = { left: newPosition, right: doc.left - 0.5 }; + shiftDistance = docSize; + } else if (newPosition > doc.right) { + move = newPosition - doc.right - 0.5; + includedRange = { left: doc.right + 0.5, right: newPosition }; + shiftDistance = -docSize; + } else { + throw new Meteor.Error('invalid-move', 'Destination must be outside the doc\'s current location'); + } + + console.log(`Moving ${doc._id} ${move} spaces, while everything between ${includedRange.left} and ${includedRange.right} is shifted by ${shiftDistance}`); + + // If the move isn't meaningfully changing the doc's location, skip + if (Math.abs(move) < 1) { + return; + } + + // Get the new parent of the doc after the move + const newParent = await collection.findOneAsync({ + 'root.id': doc.root.id, + left: { $lt: newPosition }, + right: { $gt: newPosition }, + }, { + sort: { left: -1 }, // Many ancestors match, taking the right most one gets the immediate parent + fields: { _id: 1 }, + }); + const newParentId = newParent?._id; + + // get ids of the doc and its children, so we can move them even after other docs have been + // moved around to potentially overlap the moving doc + const movedIds = await collection.find({ + 'root.id': doc.root.id, + left: { $gte: doc.left }, + right: { $lte: doc.right }, + }, { + fields: { _id: 1 } + }).mapAsync(doc => doc._id); + + // Move all the lefts and rights of documents between the current doc edge and the destination + const moveIncludedLeft = collection.updateAsync({ + 'root.id': doc.root.id, + left: { $gt: includedRange.left, $lt: includedRange.right }, + }, { + $inc: { left: shiftDistance } + }, { + multi: true + }); + const moveIncludedRight = collection.updateAsync({ + 'root.id': doc.root.id, + right: { $gt: includedRange.left, $lt: includedRange.right }, + }, { + $inc: { right: shiftDistance } + }, { + multi: true + }); + + // Move the doc and its children to the new location + const moveDocAndChildren = collection.updateAsync({ + _id: { $in: movedIds } + }, { + $inc: { + left: move, + right: move, + } + }, { + multi: true, + }); + + // Set the doc's parent to the new parentId + let changeParent: Promise | undefined; + if (newParentId !== doc.parentId) { + let update; + if (newParentId) { + update = { $set: { parentId: newParentId } }; + } else { + update = { $unset: { parentId: 1 } }; + } + changeParent = collection.updateAsync(doc._id, update); + } + return Promise.all([moveIncludedLeft, moveIncludedRight, moveDocAndChildren, changeParent]); +} + +export async function moveDocBetweenRoots(doc: TreeDoc, collection: Mongo.Collection, newRoot: Reference, newPosition: number) { + if (newRoot.id === doc.root.id) { + throw new Meteor.Error('invalid-move', 'Document is already in the given root') + } + + // get ids of the doc and its children, so we can move them even after other docs have been + // moved around to potentially overlap the moving doc + const movedIds = await collection.find({ + 'root.id': doc.root.id, + left: { $gte: doc.left }, + right: { $lte: doc.right }, + }, { + fields: { _id: 1 } + }).mapAsync(doc => doc._id); + + // Close the gap in the root we are leaving + const docSize = doc.right - doc.left + 1; + const closeGapLeft = collection.updateAsync({ + 'root.id': doc.root.id, + left: { $gt: doc.right }, + }, { + $inc: { left: -docSize }, + }, { + multi: true, + }); + const closeGapRight = collection.updateAsync({ + 'root.id': doc.root.id, + right: { $gt: doc.right }, + }, { + $inc: { right: -docSize }, + }, { + multi: true, + }); + + // Open a gap in the root we are moving to at the new location + const openGapLeft = collection.updateAsync({ + 'root.id': newRoot.id, + left: { $gt: newPosition }, + }, { + $inc: { left: -docSize }, + }, { + multi: true, + }); + const openGapRight = collection.updateAsync({ + 'root.id': newRoot.id, + right: { $gt: newPosition }, + }, { + $inc: { right: -docSize }, + }, { + multi: true, + }); + + // Move the docs to the new root and update their left and right positions to land in the gap + const moveDistance = newPosition + 0.5 - doc.left; + const moveDocs = collection.updateAsync({ + _id: { $in: movedIds }, + }, { + $set: { root: newRoot }, + $inc: { left: moveDistance, right: moveDistance }, + }, { + multi: true, + }); + + return Promise.all([closeGapLeft, closeGapRight, openGapLeft, openGapRight, moveDocs]); +} + /** * Changes the doc to be a child of the parent, and then rebuilds the nested sets of the roots * of both doc and parent + * @deprecated Use moveDocWithinRoot or moveDocBetweenRoots instead * @param doc The doc to move * @param parent The new parent of the doc, null to move the doc to the root of the tree * @param collection diff --git a/app/imports/api/properties/computedPropertySchemasIndex.js b/app/imports/api/properties/computedPropertySchemasIndex.js index 775aa504..483e41d3 100644 --- a/app/imports/api/properties/computedPropertySchemasIndex.js +++ b/app/imports/api/properties/computedPropertySchemasIndex.js @@ -1,3 +1,4 @@ +import '/imports/api/simpleSchemaConfig'; import SimpleSchema from 'simpl-schema'; import { ComputedActionSchema } from '/imports/api/properties/Actions'; import { ComputedAdjustmentSchema } from '/imports/api/properties/Adjustments';