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.

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

Sort area children by leftRightIndex #296

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
107 changes: 107 additions & 0 deletions src/__tests__/areas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import { ApolloServer } from 'apollo-server'
import muuid from 'uuid-mongodb'
import { jest } from '@jest/globals'
import MutableAreaDataSource, { createInstance as createAreaInstance } from '../model/MutableAreaDataSource.js'
import { createInstance as createClimbInstance } from '../model/MutableClimbDataSource.js'
import MutableOrganizationDataSource, { createInstance as createOrgInstance } from '../model/MutableOrganizationDataSource.js'
import { AreaType } from '../db/AreaTypes.js'
import { ClimbChangeInputType } from '../db/ClimbTypes.js'
import { OrgType, OrganizationType, OrganizationEditableFieldsType } from '../db/OrganizationTypes.js'
import { queryAPI, setUpServer } from '../utils/testUtils.js'
import { muuidToString } from '../utils/helpers.js'
Expand All @@ -22,6 +24,7 @@ describe('areas API', () => {
let usa: AreaType
let ca: AreaType
let wa: AreaType
let ak: AreaType

beforeAll(async () => {
({ server, inMemoryDB } = await setUpServer())
Expand All @@ -38,13 +41,65 @@ describe('areas API', () => {
usa = await areas.addCountry('usa')
ca = await areas.addArea(user, 'CA', usa.metadata.area_id)
wa = await areas.addArea(user, 'WA', usa.metadata.area_id)
ak = await areas.addArea(user, 'AK', usa.metadata.area_id)
})

afterAll(async () => {
await server.stop()
await inMemoryDB.close()
})

describe('mutations', () => {
it('updates sorting order of subareas and queries returns them in order', async () => {
const updateSortingOrderQuery = `
mutation ($input: [AreaSortingInput]) {
updateAreasSortingOrder(input: $input)
}
`
const updateResponse = await queryAPI({
query: updateSortingOrderQuery,
variables: {
input: [
{ areaId: wa.metadata.area_id, leftRightIndex: 3 },
{ areaId: ca.metadata.area_id, leftRightIndex: 0 },
{ areaId: ak.metadata.area_id, leftRightIndex: 10 }
]
},
userUuid
})

expect(updateResponse.statusCode).toBe(200)
const sortingOrderResult = updateResponse.body.data.updateAreasSortingOrder
expect(sortingOrderResult).toHaveLength(3)

const areaChildrenQuery = `
query area($input: ID) {
area(uuid: $input) {
children {
uuid
metadata {
leftRightIndex
}
}
}
}
`

const areaChildrenResponse = await queryAPI({
query: areaChildrenQuery,
variables: { input: usa.metadata.area_id },
userUuid
})

expect(areaChildrenResponse.statusCode).toBe(200)
const areaResult = areaChildrenResponse.body.data.area
// In leftRightIndex order
expect(areaResult.children[0]).toMatchObject({ uuid: muuidToString(ca.metadata.area_id), metadata: { leftRightIndex: 0 } })
expect(areaResult.children[1]).toMatchObject({ uuid: muuidToString(wa.metadata.area_id), metadata: { leftRightIndex: 3 } })
expect(areaResult.children[2]).toMatchObject({ uuid: muuidToString(ak.metadata.area_id), metadata: { leftRightIndex: 10 } })
})
})

describe('queries', () => {
const areaQuery = `
query area($input: ID) {
Expand Down Expand Up @@ -101,5 +156,57 @@ describe('areas API', () => {
// ca and so should not be listed.
expect(areaResult.organizations).toHaveLength(0)
})

it('returns climbs in leftRightIndex order', async () => {
const climbs = createClimbInstance()
const leftRoute: ClimbChangeInputType = {
name: 'left',
disciplines: { sport: true },
description: 'Leftmost route on the wall',
leftRightIndex: 0
}
const middleRoute: ClimbChangeInputType = {
name: 'middle',
disciplines: { sport: true },
description: 'Middle route on the wall',
leftRightIndex: 1
}
const rightRoute: ClimbChangeInputType = {
name: 'right',
disciplines: { sport: true },
description: 'Rightmost route on the wall',
leftRightIndex: 2
}
await climbs.addOrUpdateClimbs(
user,
ca.metadata.area_id,
[middleRoute, leftRoute, rightRoute]
)

const areaClimbsQuery = `
query area($input: ID) {
area(uuid: $input) {
climbs {
name
metadata {
leftRightIndex
}
}
}
}
`
const areaClimbsResponse = await queryAPI({
query: areaClimbsQuery,
variables: { input: ca.metadata.area_id },
userUuid
})
expect(areaClimbsResponse.statusCode).toBe(200)
const areaResult = areaClimbsResponse.body.data.area
console.log(areaClimbsResponse.body)
// In leftRightIndex order
expect(areaResult.climbs[0]).toMatchObject({ name: 'left', metadata: { leftRightIndex: 0 } })
expect(areaResult.climbs[1]).toMatchObject({ name: 'middle', metadata: { leftRightIndex: 1 } })
expect(areaResult.climbs[2]).toMatchObject({ name: 'right', metadata: { leftRightIndex: 2 } })
})
})
})
10 changes: 8 additions & 2 deletions src/model/AreaDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { getAreaModel, getMediaModel, getMediaObjectModel } from '../db/index.js
import { AreaType } from '../db/AreaTypes'
import { GQLFilter, AreaFilterParams, PathTokenParams, LeafStatusParams, ComparisonFilterParams, StatisticsType, CragsNear, BBoxType } from '../types'
import { getClimbModel } from '../db/ClimbSchema.js'
import { ClimbGQLQueryType } from '../db/ClimbTypes.js'
import { ClimbGQLQueryType, ClimbType } from '../db/ClimbTypes.js'
import { logger } from '../logger.js'

export default class AreaDataSource extends MongoDataSource<AreaType> {
Expand Down Expand Up @@ -108,6 +108,12 @@ export default class AreaDataSource extends MongoDataSource<AreaType> {
$set: {
'climbs.gradeContext': '$gradeContext' // manually set area's grade context to climb
}
},
{
$set: {
climbs: { $sortArray: { input: '$climbs', sortBy: { 'metadata.left_right_index': 1 } } },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$sortArray requires Mongo 5.2 and up. Otherwise we need something like this: https://stackoverflow.com/questions/15388127/mongodb-sort-inner-array

children: { $sortArray: { input: '$children', sortBy: { 'metadata.leftRightIndex': 1 } } }
Copy link
Contributor

@vnugent vnugent May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you confirm children[] contains actual area objects? I think it only contains a list of area _id's. When we ask for an area.children[] in GQL, we're making another DB query here:
https://github.com/OpenBeta/openbeta-graphql/blob/develop/src/graphql/resolvers.ts#L203

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah you're right! I've not tested this far because it throws a "No such operation as $sortArray" error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we did a $lookup for children above this $set so that we have the full object?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we did a $lookup for children above this $set so that we have the full object?

I thought the query would get too wildly complicated for a small performance gain, whereas the GQL field resolution way in my comment above is more elegant and simpler (but requires an additional query).

}
}
])

Expand All @@ -117,7 +123,7 @@ export default class AreaDataSource extends MongoDataSource<AreaType> {
throw new Error(`Area ${uuid.toUUID().toString()} not found.`)
}

async findManyClimbsByUuids (uuidList: muuid.MUUID[]): Promise<any> {
async findManyClimbsByUuids (uuidList: muuid.MUUID[]): Promise<ClimbType[]> {
const rs = await this.climbModel.find().where('_id').in(uuidList)
return rs
}
Expand Down