fix(import): corecctly handle id conversion

src/__tests__/bulkImport.test.ts

@@ -4,21 +4,27 @@ import express from "express";
 import {InMemoryDB} from "../utils/inMemoryDB.js";
 import {queryAPI, setUpServer} from "../utils/testUtils.js";
 import {muuidToString} from "../utils/helpers.js";
-import MutableAreaDataSource from "../model/MutableAreaDataSource.js";
 import exampleImportData from './import-example.json' assert {type: 'json'};
 import {AreaType} from "../db/AreaTypes.js";
 import {BulkImportResultType} from "../db/BulkImportTypes.js";
 import MutableClimbDataSource from "../model/MutableClimbDataSource.js";
+import BulkImportDataSource from "../model/BulkImportDataSource.js";
 
 describe('bulkImportAreas', () => {
   const query = `
     mutation bulkImportAreas($input: BulkImportInput!) {
       bulkImportAreas(input: $input) {
         addedAreas {
           uuid
+          metadata {
+            area_id
+          }
         }
         updatedAreas {
           uuid
+          metadata {
+            area_id
+          }
         }
         addedOrUpdatedClimbs {
           id
@@ -34,7 +40,7 @@ describe('bulkImportAreas', () => {
   let inMemoryDB: InMemoryDB
   let testArea: AreaType
 
-  let areas: MutableAreaDataSource
+  let bulkImport: BulkImportDataSource
   let climbs: MutableClimbDataSource
 
   beforeAll(async () => {
@@ -44,14 +50,14 @@ describe('bulkImportAreas', () => {
     user = muuid.mode('relaxed').v4()
     userUuid = muuidToString(user)
 
-    areas = MutableAreaDataSource.getInstance()
+    bulkImport = BulkImportDataSource.getInstance()
     climbs = MutableClimbDataSource.getInstance()
   })
 
   beforeEach(async () => {
     await inMemoryDB.clear()
-    await areas.addCountry('usa')
-    testArea = await areas.addArea(user, "Test Area", null, "us")
+    await bulkImport.addCountry('usa')
+    testArea = await bulkImport.addArea(user, "Test Area", null, "us")
   })
 
   afterAll(async () => {
@@ -106,25 +112,25 @@ describe('bulkImportAreas', () => {
           areas: [
             ...exampleImportData.areas,
             {
-              uuid: testArea.metadata.area_id.toUUID().toString(),
+              uuid: testArea.metadata.area_id,
               areaName: "Updated Test Area",
             }
           ]
         }
       }
     });
-    expect(res.status).toBe(200)
+    expect(res.body.errors).toBeFalsy()
 
-    const result = res.body.data as BulkImportResultType
+    const result = res.body.data.bulkImportAreas as BulkImportResultType
     expect(result.addedAreas.length).toBe(4)
 
-    const committedAreas = await Promise.all(result.addedAreas.map((area) => areas.findOneAreaByUUID(area.metadata.area_id)));
+    const committedAreas = await Promise.all(result.addedAreas.map((area) => bulkImport.findOneAreaByUUID(muuid.from(area.metadata.area_id))));
     expect(committedAreas.length).toBe(4);
 
     const committedClimbs = await Promise.all(result.addedOrUpdatedClimbs.map((climb) => climbs.findOneClimbByMUUID(climb._id)));
     expect(committedClimbs.length).toBe(2);
 
-    const updatedAreas = await Promise.all(result.updatedAreas.map((area) => areas.findOneAreaByUUID(area.metadata.area_id)));
+    const updatedAreas = await Promise.all(result.updatedAreas.map((area) => bulkImport.findOneAreaByUUID(muuid.from(area.metadata.area_id))));
     expect(updatedAreas.length).toBe(1);
     expect(updatedAreas[0].area_name).toBe("Updated Test Area");
   })

src/graphql/schema/AreaEdit.gql

@@ -29,7 +29,7 @@ type Mutation {
   You can start at any point in the tree given a valid parent area with its id.
   If starting at the root level, the countryCode must be provided.
   """
-  bulkImportAreas(input: BulkImportInput): [BulkImportResult]
+  bulkImportAreas(input: BulkImportInput): BulkImportResult
 }
 
 input DestinationFlagInput {
@@ -71,7 +71,7 @@ input BulkImportAreaInput {
   areaName: String
   "An optional description of the area. If provided with an id, the description will be updated"
   description: String
-  "Only relevant for the top most areas of a country. Get a list of valid countries by querying the countries field"
+  "Only relevant for the top most areas of a country. Get a list of valid countries by using the countries query"
   countryCode: String
   "The grade context of the area. Every area that has climbs must have a valid grade context"
   gradeContext: String

src/model/BulkImportDataSource.ts

@@ -1,5 +1,5 @@
 import MutableAreaDataSource from './MutableAreaDataSource.js'
-import mongoose from 'mongoose'
+import mongoose, { ClientSession } from 'mongoose'
 import { withTransaction } from '../utils/helpers.js'
 import muuid, { MUUID } from 'uuid-mongodb'
 import { AreaType } from '../db/AreaTypes.js'
@@ -48,7 +48,9 @@ export default class BulkImportDataSource extends MutableAreaDataSource {
       logger.error('bulk import failed', e)
       throw e
     } finally {
-      await session.endSession()
+      if (!session.hasEnded) {
+        await session.endSession()
+      }
     }
   }
 
@@ -71,7 +73,7 @@ export default class BulkImportDataSource extends MutableAreaDataSource {
       if (areaNode.uuid !== undefined && areaNode.uuid !== null) {
         area = await this.updateAreaWith({
           user,
-          areaUuid: areaNode.uuid,
+          areaUuid: muuid.from(areaNode.uuid),
           document: {
             areaName: areaNode.areaName,
             description: areaNode.description,
@@ -118,16 +120,17 @@ export default class BulkImportDataSource extends MutableAreaDataSource {
         }
       }
       if (areaNode.climbs !== undefined) {
-        result.addedOrUpdatedClimbs.push(...(await climbs?.addOrUpdateClimbsWith({
+        const addedOrUpdatedClimbs = await Promise.all(await climbs?.addOrUpdateClimbsWith({
           userId: user,
           parentId: area.metadata.area_id,
           changes: [...areaNode.climbs.map(this.toClimbChangeInputType) ?? []],
           session
-        }).then((climbIds) => climbIds.map(async (id) =>
-          await climbs?.findOneClimbByMUUID(muuid.from(id)))
+        }).then((climbIds) => climbIds
+          .map((id) => climbs?.climbModel.findById(muuid.from(id)).session(session as ClientSession))) ?? [])
+        result.addedOrUpdatedClimbs.push(...addedOrUpdatedClimbs
           .filter((climb) => climb !== null)
-          .map((climb) => climb as unknown as ClimbType))
-        ))
+          .map((climb) => climb as unknown as ClimbType)
+        )
       }
       return result
     }

src/model/MutableAreaDataSource.ts

@@ -158,20 +158,26 @@ export default class MutableAreaDataSource extends AreaDataSource {
       throw new Error(`Adding area "${areaName}" failed. Must provide parent Id or country code`)
     }
 
-    let uuid: MUUID
+    let parentId: MUUID
     if (parentUuid != null) {
-      uuid = parentUuid
+      parentId = parentUuid
     } else if (countryCode != null) {
-      uuid = countryCode2Uuid(countryCode)
+      parentId = countryCode2Uuid(countryCode)
     } else {
       throw new Error(`Adding area "${areaName}" failed. Unable to determine parent id or country code`)
     }
 
     const session = sessionCtx ?? await this.areaModel.startSession()
-    if (session.inTransaction()) {
-      return await this._addArea(session, user, areaName, uuid, experimentalAuthor, isLeaf, isBoulder)
-    } else {
-      return await withTransaction(session, async () => await this._addArea(session, user, areaName, uuid, experimentalAuthor, isLeaf, isBoulder))
+    try {
+      if (session.inTransaction()) {
+        return await this._addArea(session, user, areaName, parentId, experimentalAuthor, isLeaf, isBoulder)
+      } else {
+        return await withTransaction(session, async () => await this._addArea(session, user, areaName, parentId, experimentalAuthor, isLeaf, isBoulder))
+      }
+    } finally {
+      if (sessionCtx == null) {
+        await session.endSession()
+      }
     }
   }
 
@@ -338,6 +344,7 @@ export default class MutableAreaDataSource extends AreaDataSource {
    * @param user
    * @param areaUuid Area uuid to be updated
    * @param document New fields
+   * @param sessionCtx optional existing session to use for the transactions
    * @returns Newly updated area
    */
   async updateArea (user: MUUID, areaUuid: MUUID, document: AreaEditableFieldsType, sessionCtx?: ClientSession): Promise<AreaType | null> {
@@ -349,7 +356,7 @@ export default class MutableAreaDataSource extends AreaDataSource {
       const area = await this.areaModel.findOne(filter).session(session)
 
       if (area == null) {
-        throw new Error('Area update error.  Reason: Area not found.')
+        throw new Error(`Area update error. Reason: Area with id ${areaUuid.toString()} not found.`)
       }
 
       const {
@@ -365,23 +372,23 @@ export default class MutableAreaDataSource extends AreaDataSource {
       } = document
 
       // See https://github.com/OpenBeta/openbeta-graphql/issues/244
-      let experimentaAuthorId: MUUID | null = null
+      let experimentalAuthorId: MUUID | null = null
       if (experimentalAuthor != null) {
-        experimentaAuthorId = await this.experimentalUserDataSource.updateUser(session, experimentalAuthor.displayName, experimentalAuthor.url)
+        experimentalAuthorId = 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,
+        user: experimentalAuthorId ?? user,
         historyId: change._id,
         prevHistoryId: area._change?.historyId._id,
         operation: opType,
         seq: 0
       }
       area.set({ _change })
-      area.updatedBy = experimentaAuthorId ?? user
+      area.updatedBy = experimentalAuthorId ?? user
 
       if (area.pathTokens.length === 1) {
         if (areaName != null || shortCode != null) throw new Error(`[${area.area_name}]: Area update error. Reason: Updating country name or short code is not allowed.`)
@@ -435,10 +442,16 @@ export default class MutableAreaDataSource extends AreaDataSource {
     }
 
     const session = sessionCtx ?? await this.areaModel.startSession()
-    if (session.inTransaction()) {
-      return await _updateArea(session, user, areaUuid, document)
-    } else {
-      return await withTransaction(session, async () => await _updateArea(session, user, areaUuid, document))
+    try {
+      if (session.inTransaction()) {
+        return await _updateArea(session, user, areaUuid, document)
+      } else {
+        return await withTransaction(session, async () => await _updateArea(session, user, areaUuid, document))
+      }
+    } finally {
+      if (sessionCtx == null) {
+        await session.endSession()
+      }
     }
   }
 

src/server.ts

@@ -20,6 +20,7 @@ import type { DataSources } from 'apollo-server-core/dist/graphqlOptions'
 import UserDataSource from './model/UserDataSource.js'
 import express from 'express'
 import * as http from 'http'
+import BulkImportDataSource from './model/BulkImportDataSource'
 
 export async function createServer (): Promise<{ app: express.Application, server: ApolloServer }> {
   const schema = applyMiddleware(
@@ -29,6 +30,7 @@ export async function createServer (): Promise<{ app: express.Application, serve
   const dataSources: () => DataSources<Context> = () => ({
     climbs: MutableClimbDataSource.getInstance(),
     areas: MutableAreaDataSource.getInstance(),
+    bulkImport: BulkImportDataSource.getInstance(),
     organizations: MutableOrgDS.getInstance(),
     ticks: TickDataSource.getInstance(),
     history: ChangeLogDataSource.getInstance(),