Skip to content

Commit

Permalink
Breadcrumbs issue #213 fix (#314)
Browse files Browse the repository at this point in the history
* refactor: use mongoose 'populate()' to hydrate children array
* added basic unit test for (#213) - verify pathToken change through tree
* simplified array indexing since the value is constant, passes tests
  • Loading branch information
andrew-jp committed Jul 4, 2023
1 parent e757f9f commit ccc3b6a
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 19 deletions.
78 changes: 59 additions & 19 deletions src/model/MutableAreaDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import { createInstance as createExperimentalUserDataSource } from '../model/Exp

isoCountries.registerLocale(enJson)

type AreaDocumnent = mongoose.Document<unknown, any, AreaType> & AreaType

export default class MutableAreaDataSource extends AreaDataSource {
experimentalUserDataSource = createExperimentalUserDataSource()

Expand Down Expand Up @@ -293,6 +295,25 @@ export default class MutableAreaDataSource extends AreaDataSource {

const { areaName, description, shortCode, isDestination, isLeaf, isBoulder, lat, lng, experimentalAuthor } = document

// See https://github.com/OpenBeta/openbeta-graphql/issues/244
let experimentaAuthorId: MUUID | null = null
if (experimentalAuthor != null) {
experimentaAuthorId = await this.experimentalUserDataSource.updateUser(session, experimentalAuthor.displayName, experimentalAuthor.url)
}

const opType = OperationType.updateArea
const change = await changelogDataSource.create(session, user, opType)

const _change: ChangeRecordMetadataType = {
user: experimentaAuthorId ?? user,
historyId: change._id,
prevHistoryId: area._change?.historyId._id,
operation: opType,
seq: 0
}
area.set({ _change })
area.updatedBy = experimentaAuthorId ?? user

if (area.pathTokens.length === 1) {
if (areaName != null || shortCode != null) throw new Error('Area update error. Reason: Updating country name or short code is not allowed.')
}
Expand All @@ -301,7 +322,14 @@ export default class MutableAreaDataSource extends AreaDataSource {
throw new Error('Area update error. Reason: Updating leaf or boulder status of an area with subareas is not allowed.')
}

if (areaName != null) area.set({ area_name: sanitizeStrict(areaName) })
if (areaName != null) {
const sanitizedName = sanitizeStrict(areaName)
area.set({ area_name: sanitizedName })

// change our pathTokens
await this.updatePathTokens(session, _change, area, sanitizedName)
}

if (shortCode != null) area.set({ shortCode: shortCode.toUpperCase() })
if (isDestination != null) area.set({ 'metadata.isDestination': isDestination })
if (isLeaf != null) area.set({ 'metadata.leaf': isLeaf })
Expand All @@ -323,24 +351,6 @@ export default class MutableAreaDataSource extends AreaDataSource {
})
}

// See https://github.com/OpenBeta/openbeta-graphql/issues/244
let experimentaAuthorId: MUUID | null = null
if (experimentalAuthor != null) {
experimentaAuthorId = await this.experimentalUserDataSource.updateUser(session, experimentalAuthor.displayName, experimentalAuthor.url)
}

const opType = OperationType.updateArea
const change = await changelogDataSource.create(session, user, opType)

const _change: ChangeRecordMetadataType = {
user: experimentaAuthorId ?? user,
historyId: change._id,
prevHistoryId: area._change?.historyId._id,
operation: opType,
seq: 0
}
area.set({ _change })
area.updatedBy = experimentaAuthorId ?? user
const cursor = await area.save()
return cursor.toObject()
}
Expand All @@ -358,6 +368,36 @@ export default class MutableAreaDataSource extends AreaDataSource {
return ret
}

/**
* Update path tokens
* @param session Mongoose session
* @param changeRecord Changeset metadata
* @param area area to update
* @param newAreaName new area name
* @param depth tree depth
*/
async updatePathTokens (session: ClientSession, changeRecord: ChangeRecordMetadataType, area: AreaDocumnent, newAreaName: string, changeIndex: number = -1): Promise<void> {
if (area.pathTokens.length > 1) {
if (changeIndex === -1) { changeIndex = area.pathTokens.length - 1 }

const newPath = [...area.pathTokens]
newPath[changeIndex] = newAreaName
area.set({ pathTokens: newPath })
area.set({ _change: changeRecord })
await area.save({ session })

// hydrate children_ids array with actual area documents
await area.populate('children')

await Promise.all(area.children.map(async childArea => {
// TS complains about ObjectId type
// Fix this when we upgrade Mongoose library
// @ts-expect-error
await this.updatePathTokens(session, changeRecord, childArea, newAreaName, changeIndex)
}))
}
}

/**
*
* @param user user id
Expand Down
104 changes: 104 additions & 0 deletions src/model/__tests__/updateAreas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,4 +299,108 @@ describe('Areas', () => {
await areas.updateSortingOrder(testUser,
[{ ...change3, leftRightIndex: -1 }, { ...change4, leftRightIndex: -1 }])
})

it('should update self and childrens pathTokens', async () => {
await areas.addCountry('JP')
const a1 = await areas.addArea(testUser, 'Parent', null, 'JP')
const b1 = await areas.addArea(testUser, 'B1', a1.metadata.area_id)
const b2 = await areas.addArea(testUser, 'B2', a1.metadata.area_id)
const c1 = await areas.addArea(testUser, 'C1', b1.metadata.area_id)
const c2 = await areas.addArea(testUser, 'C2', b1.metadata.area_id)
const c3 = await areas.addArea(testUser, 'C3', b2.metadata.area_id)
const e1 = await areas.addArea(testUser, 'E1', c3.metadata.area_id)

let a1Actual = await areas.findOneAreaByUUID(a1.metadata.area_id)
expect(a1Actual).toEqual(
expect.objectContaining({
area_name: 'Parent',
pathTokens: ['Japan', 'Parent']
}))

let b1Actual = await areas.findOneAreaByUUID(b1.metadata.area_id)
expect(b1Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Parent', 'B1']
}))

let b2Actual = await areas.findOneAreaByUUID(b2.metadata.area_id)
expect(b2Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Parent', 'B2']
}))

let c1Actual = await areas.findOneAreaByUUID(c1.metadata.area_id)
expect(c1Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Parent', 'B1', 'C1']
}))

let c2Actual = await areas.findOneAreaByUUID(c2.metadata.area_id)
expect(c2Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Parent', 'B1', 'C2']
}))

let c3Actual = await areas.findOneAreaByUUID(c3.metadata.area_id)
expect(c3Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Parent', 'B2', 'C3']
}))

let e1Actual = await areas.findOneAreaByUUID(e1.metadata.area_id)
expect(e1Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Parent', 'B2', 'C3', 'E1']
}))

// Update
const doc1: AreaEditableFieldsType = {
areaName: 'Test Name'
}
await areas.updateArea(testUser, a1?.metadata.area_id, doc1)

// Verify
a1Actual = await areas.findOneAreaByUUID(a1.metadata.area_id)
expect(a1Actual).toEqual(
expect.objectContaining({
area_name: 'Test Name',
pathTokens: ['Japan', 'Test Name']
}))

b1Actual = await areas.findOneAreaByUUID(b1.metadata.area_id)
expect(b1Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Test Name', 'B1']
}))

b2Actual = await areas.findOneAreaByUUID(b2.metadata.area_id)
expect(b2Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Test Name', 'B2']
}))

c1Actual = await areas.findOneAreaByUUID(c1.metadata.area_id)
expect(c1Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Test Name', 'B1', 'C1']
}))

c2Actual = await areas.findOneAreaByUUID(c2.metadata.area_id)
expect(c2Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Test Name', 'B1', 'C2']
}))

c3Actual = await areas.findOneAreaByUUID(c3.metadata.area_id)
expect(c3Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Test Name', 'B2', 'C3']
}))

e1Actual = await areas.findOneAreaByUUID(e1.metadata.area_id)
expect(e1Actual).toEqual(
expect.objectContaining({
pathTokens: ['Japan', 'Test Name', 'B2', 'C3', 'E1']
}))
})
})

0 comments on commit ccc3b6a

Please sign in to comment.