fix: remove uniqueness requirement for area's L-R indices (#376)

Author: vnugent

db-migrations/0005-area-sorting.js

@@ -0,0 +1,5 @@
+/**
+ * Issue: 375
+ */
+
+db.areas.dropIndexes('metadata.leftRightIndex_1')

src/db/AreaSchema.ts

@@ -118,8 +118,9 @@ export const AreaSchema = new Schema<AreaType>({
 }, { timestamps: true })
 
 AreaSchema.index({ _deleting: 1 }, { expireAfterSeconds: 0 })
-AreaSchema.index({ 'metadata.leftRightIndex': 1 }, {
-  unique: true,
+AreaSchema.index({
+  'metadata.leftRightIndex': 1
+}, {
   partialFilterExpression: {
     'metadata.leftRightIndex': {
       $gt: -1

src/model/MutableAreaDataSource.ts

@@ -427,21 +427,6 @@ export default class MutableAreaDataSource extends AreaDataSource {
       const opType = OperationType.orderAreas
       const change = await changelogDataSource.create(session, user, opType)
       const updates: any[] = []
-      let expectedOpCount = input.length
-
-      // Clear existing indices so we can re-order without running into duplicate key errors.
-      if (input.some(i => i.leftRightIndex >= 0)) {
-        updates.push({
-          updateMany: {
-            filter: { 'metadata.area_id': { $in: input.map(i => muuid.from(i.areaId)) } },
-            update: {
-              $set: { 'metadata.leftRightIndex': -1 }
-              // Don't record change since this is an intermediate step.
-            }
-          }
-        })
-        expectedOpCount = expectedOpCount * 2
-      }
 
       input.forEach(({ areaId, leftRightIndex }, index) => {
         updates.push({
@@ -465,7 +450,7 @@ export default class MutableAreaDataSource extends AreaDataSource {
 
       const rs = (await this.areaModel.bulkWrite(updates, { session })).toJSON()
 
-      if (rs.ok === 1 && rs.nMatched === rs.nModified && rs.nMatched === expectedOpCount) {
+      if (rs.ok === 1 && rs.nMatched === rs.nModified) {
         return input.map(item => item.areaId)
       } else {
         throw new Error(`Expect to update ${input.length} areas but found ${rs.nMatched}.`)

src/model/__tests__/updateAreas.ts

@@ -286,28 +286,6 @@ describe('Areas', () => {
           leftRightIndex: change2.leftRightIndex
         })
       }))
-
-    // Able to overwrite existing leftRightIndices without duplicate key errors
-    const change3: UpdateSortingOrderType = {
-      areaId: a1.metadata.area_id.toUUID().toString(),
-      leftRightIndex: 9
-    }
-    const change4: UpdateSortingOrderType = {
-      areaId: a2.metadata.area_id.toUUID().toString(),
-      leftRightIndex: 10
-    }
-
-    await expect(areas.updateSortingOrder(testUser, [change3, change4])).resolves.toStrictEqual(
-      [a1.metadata.area_id.toUUID().toString(), a2.metadata.area_id.toUUID().toString()])
-
-    // Make sure we can't have duplicate leftToRight indices >= 0
-    await expect(
-      areas.updateSortingOrder(testUser, [{ ...change3, leftRightIndex: change4.leftRightIndex }]))
-      .rejects.toThrowError(/E11000/)
-
-    // But we can have duplicate indices < 0 to indicate unsorted
-    await areas.updateSortingOrder(testUser,
-      [{ ...change3, leftRightIndex: -1 }, { ...change4, leftRightIndex: -1 }])
   })
 
   it('should update self and childrens pathTokens', async () => {