fix pr comments

Author: ryanslatten

service/src/app.impl/observations/app.impl.observations.ts

@@ -23,14 +23,14 @@ export function SaveObservation(permissionService: api.ObservationPermissionServ
   return async function saveObservation(req: api.SaveObservationRequest): ReturnType<api.SaveObservation> {
     const repo = req.context.observationRepository
     const mod = req.observation
-    const before = await repo.findById(mod.id)
-    const denied = before ?
+    const existingObservation = await repo.findById(mod.id)
+    const denied = existingObservation ?
       await permissionService.ensureUpdateObservationPermission(req.context) :
       await permissionService.ensureCreateObservationPermission(req.context)
     if (denied) {
       return AppResponse.error(denied)
     }
-    const obs = await prepareObservationMod(mod, before, req.context)
+    const obs = await prepareObservationMod(mod, existingObservation, req.context)
     if (obs instanceof MageError) {
       return AppResponse.error(obs)
     }
@@ -169,33 +169,33 @@ export function registerDeleteRemovedAttachmentsHandler(domainEvents: EventEmitt
  * an `isPending` property.  That should be reasonable to implement, but no
  * time now, as usual.
  */
-async function prepareObservationMod(mod: api.ExoObservationMod, before: Observation | null, context: api.ObservationRequestContext): Promise<Observation | InvalidInputError> {
+async function prepareObservationMod(mod: api.ExoObservationMod, observationToUpdate: Observation | null, context: api.ObservationRequestContext): Promise<Observation | InvalidInputError> {
   const event = context.mageEvent
   const repo = context.observationRepository
-  const modAttrs = baseObservationAttrsForMod(mod, before, context)
+  const modAttrs = baseObservationAttrsForMod(mod, observationToUpdate, context)
   // first get new form entry ids so new attachments have a proper id to reference
   const [removedFormEntries, newFormEntries] = mod.properties.forms.reduce(([removed, added], entryMod) => {
-    if (entryMod.id && before?.formEntryForId(entryMod.id)) {
+    if (entryMod.id && observationToUpdate?.formEntryForId(entryMod.id)) {
       removed.delete(entryMod.id)
     }
     else {
       added.push(entryMod)
     }
     return [removed, added]
-  }, [new Map(before?.formEntries.map(x => [x.id, x]) || []), [] as api.ExoFormEntryMod[]])
+  }, [new Map(observationToUpdate?.formEntries.map(x => [x.id, x]) || []), [] as api.ExoFormEntryMod[]])
   const newFormEntryIds = newFormEntries.length ? await repo.nextFormEntryIds(newFormEntries.length) : []
   newFormEntries.forEach(x => x.id = newFormEntryIds.shift())
   const attachmentExtraction = extractAttachmentModsFromFormEntries(mod, event)
   modAttrs.properties.forms = attachmentExtraction.formEntries
   const attachmentMods = attachmentExtraction.attachmentMods
   const addCount = attachmentMods.reduce((count, x) => x.action === api.AttachmentModAction.Add ? count + 1 : count, 0)
   const attachmentIds = addCount ? await repo.nextAttachmentIds(addCount) : []
-  const afterRemovedFormEntryAttachments = before?.attachments.reduce((obs, attachment) => {
+  const afterRemovedFormEntryAttachments = observationToUpdate?.attachments.reduce((obs, attachment) => {
     if (removedFormEntries.has(attachment.observationFormId)) {
       return removeAttachment(obs, attachment.id) as Observation
     }
     return obs
-  }, before)
+  }, observationToUpdate)
   if (afterRemovedFormEntryAttachments) {
     modAttrs.attachments = afterRemovedFormEntryAttachments.attachments
   }
@@ -206,18 +206,17 @@ async function prepareObservationMod(mod: api.ExoObservationMod, before: Observa
     if (obs instanceof MageError) {
       return obs
     }
-    const mod =
-      attachmentMod.action === api.AttachmentModAction.Add ?
-        addAttachment(obs, attachmentIds.shift() as string, attachmentMod.fieldName, attachmentMod.formEntryId, attachmentCreateAttrsForMod(attachmentMod)) :
-        attachmentMod.action === api.AttachmentModAction.Delete ?
-          removeAttachment(obs, attachmentMod.id) :
-          null
+    let mod;
+    if (attachmentMod.action === api.AttachmentModAction.Add) {
+      mod = addAttachment(obs, attachmentIds.shift() as string, attachmentMod.fieldName, attachmentMod.formEntryId, attachmentCreateAttrsForMod(attachmentMod))
+    } else if (attachmentMod.action === api.AttachmentModAction.Delete) {
+      mod = removeAttachment(obs, attachmentMod.id)
+    } else {
+      return invalidInput(`invalid attachment action: ${attachmentMod.action}`)
+    }
     if (mod instanceof Observation) {
       return mod
     }
-    if (mod === null) {
-      return invalidInput(`invalid attachment action: ${attachmentMod.action}`)
-    }
     const message = `error adding attachment on observation ${obs.id}`
     return invalidInput(`${message}: ${String(mod)}`)
   }, afterFormEntriesRemoved)
@@ -229,22 +228,22 @@ async function prepareObservationMod(mod: api.ExoObservationMod, before: Observa
  * existing observation.  The result will not include form entries and
  * attachments, which require separate processing to resolve IDs and actions.
  * @param mod the modifications from an external client
- * @param before the observation to update, or null if none exists
+ * @param observationToUpdate the observation to update, or null if none exists
  */
-function baseObservationAttrsForMod(mod: api.ExoObservationMod, before: Observation | null, context: api.ObservationRequestContext): ObservationAttrs {
+function baseObservationAttrsForMod(mod: api.ExoObservationMod, observationToUpdate: Observation | null, context: api.ObservationRequestContext): ObservationAttrs {
   const attrs: ObservationAttrs = {
     id: mod.id,
     eventId: context.mageEvent.id,
-    userId: before ? before.userId : context.userId,
-    deviceId: before ? before.deviceId : context.deviceId,
-    createdAt: before ? before.createdAt : new Date(),
+    userId: observationToUpdate ? observationToUpdate.userId : context.userId,
+    deviceId: observationToUpdate ? observationToUpdate.deviceId : context.deviceId,
+    createdAt: observationToUpdate ? observationToUpdate.createdAt : new Date(),
     lastModified: new Date(),
     geometry: mod.geometry,
     type: 'Feature',
-    states: before ? before.states : [],
-    bbox: mod.bbox || before?.bbox,
-    favoriteUserIds: before?.favoriteUserIds || [],
-    important: before?.important,
+    states: observationToUpdate ? observationToUpdate.states : [],
+    bbox: mod.bbox || observationToUpdate?.bbox,
+    favoriteUserIds: observationToUpdate?.favoriteUserIds || [],
+    important: observationToUpdate?.important,
     properties: {
       // TODO: should timestamp be optional on the mod object?
       timestamp: mod.properties.timestamp,
@@ -253,9 +252,9 @@ function baseObservationAttrsForMod(mod: api.ExoObservationMod, before: Observat
     attachments: [],
     noGeometry: !!mod.noGeometry,
   }
-  assignFirstDefined('accuracy', attrs.properties, mod.properties, before?.properties)
-  assignFirstDefined('delta', attrs.properties, mod.properties, before?.properties)
-  assignFirstDefined('provider', attrs.properties, mod.properties, before?.properties)
+  assignFirstDefined('accuracy', attrs.properties, mod.properties, observationToUpdate?.properties)
+  assignFirstDefined('delta', attrs.properties, mod.properties, observationToUpdate?.properties)
+  assignFirstDefined('provider', attrs.properties, mod.properties, observationToUpdate?.properties)
   return attrs
 }
 

service/src/models/event.js

@@ -76,7 +76,7 @@ const EventSchema = new Schema({
   _id: { type: Number, required: true },
   name: { type: String, required: true, unique: true },
   description: { type: String, required: false },
-  noGeometry: { type: Boolean, default: false },
+  // noGeometry: { type: Boolean, default: false },
   complete: { type: Boolean },
   collectionName: { type: String, required: true },
   teamIds: [{ type: Schema.Types.ObjectId, ref: 'Team' }],