Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Jan 3, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions db-migrations/0005-area-sorting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/**
* Issue: 375
*/

db.areas.dropIndexes('metadata.leftRightIndex_1')
5 changes: 3 additions & 2 deletions src/db/AreaSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 1 addition & 16 deletions src/model/MutableAreaDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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}.`)
Expand Down
22 changes: 0 additions & 22 deletions src/model/__tests__/updateAreas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading