Make sure all annotations use object for target property (#173)

gbv · May 6, 2022 · 51812a0 · 51812a0
1 parent 76a8011
commit 51812a0
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 29 deletions.
diff --git a/services/annotations.js b/services/annotations.js
@@ -152,19 +152,17 @@ class MappingService {
         if (config.env === "production") {
           annotation.id = annotation.id.replace("http:", "https:")
         }
-        // If it annotates a mapping from same instance, then add state to target
+        // Change target to object and add mapping content identifier if possible
         const target = _.get(annotation, "target.id", annotation.target)
-        if (target && target.startsWith && target.startsWith(config.baseUrl + "mappings/") && !_.get(annotation, "target.state.id")) {
+        if (!_.get(annotation, "target.state.id")) {
           const mapping = await Mapping.findOne({ uri: target })
           const contentId = mapping && (mapping.identifier || []).find(id => id.startsWith("urn:jskos:mapping:content:"))
-          if (contentId) {
-            annotation.target = {
-              id: target,
-              state: {
-                id: contentId,
-              },
-            }
-          }
+          annotation.target = contentId ? {
+            id: target,
+            state: {
+              id: contentId,
+            },
+          } : { id: target }
         }
 
         return annotation
@@ -209,6 +207,11 @@ class MappingService {
     annotation.id = existing.id
     annotation._id = existing._id
 
+    // Change target property to object if necessary
+    if (_.isString(annotation.target)) {
+      annotation.target = { id: annotation.target }
+    }
+
     const result = await Annotation.replaceOne({ _id: existing._id }, annotation)
     if (result.acknowledged && result.matchedCount) {
       return annotation
@@ -232,6 +235,10 @@ class MappingService {
     _.unset(annotation, "id")
     // Use lodash merge to merge annotations
     _.merge(existing, annotation)
+    // Change target property to object if necessary
+    if (_.isString(annotation.target)) {
+      annotation.target = { id: annotation.target }
+    }
     // Validate mapping
     if (!validate.annotation(annotation)) {
       throw new InvalidBodyError()

diff --git a/services/mappings.js b/services/mappings.js
@@ -263,7 +263,7 @@ class MappingService {
           $lookup: {
             from: "annotations",
             localField: "uri",
-            foreignField: "target",
+            foreignField: "target.id",
             as: "annotations",
           },
         },
@@ -296,7 +296,7 @@ class MappingService {
         {
           $lookup: {
             from: "mappings",
-            localField: "target",
+            localField: "target.id",
             foreignField: "uri",
             as: "mappings",
           },

diff --git a/test/test.js b/test/test.js
@@ -1854,7 +1854,7 @@ describe("Express Server", () => {
           // Save id for later use
           annotation.id = res.body.id
           res.body.creator.should.be.eql({ id: user.uri, name: user.name }) // Creator gets decoded from base64
-          res.body.target.should.be.eql(annotation.target)
+          _.get(res.body, "target.id", res.body.target).should.be.eql(annotation.target)
           res.body.motivation.should.be.eql(annotation.motivation)
           res.body.bodyValue.should.be.eql(annotation.bodyValue)
           done()
@@ -2105,7 +2105,7 @@ describe("Express Server", () => {
           res.body.should.be.a("object")
           res.body.id.should.be.a("string")
           res.body.creator.should.be.eql({ id: userWithModerating.uri, name: userWithModerating.name }) // Creator gets decoded from base64
-          res.body.target.should.be.eql(annotationModerating.target)
+          _.get(res.body, "target.id", res.body.target).should.be.eql(annotationModerating.target)
           res.body.motivation.should.be.eql(annotationModerating.motivation)
           done()
         })

diff --git a/utils/version.js b/utils/version.js
@@ -187,29 +187,35 @@ const upgrades = {
     await annotationService.createIndexes()
     console.log("... done.")
 
-    console.log("- Update annotations to include mapping state in target property...")
+    console.log("- Annotations will be updated to use an object for property `target` and to include mapping state if possible...")
+    const ok = await yesno({
+      question: "Is that okay?",
+      defaultValue: false,
+    })
+    if (!ok) {
+      throw new Error("Aborted due to missing user confirmation.")
+    }
     const Mapping = require("../models/mappings")
     const Annotation = require("../models/annotations")
 
     let updatedCount = 0
     const annotations = await Annotation.find({ "target.state.id": { $exists: false } }).exec()
     for (const annotation of annotations) {
       const target = _.get(annotation, "target.id", annotation.target)
-      if (target && target.startsWith && target.startsWith(config.baseUrl + "mappings/")) {
-        const mapping = await Mapping.findOne({ uri: target })
-        const contentId = mapping && (mapping.identifier || []).find(id => id.startsWith("urn:jskos:mapping:content:"))
-        if (contentId) {
-          await Annotation.updateOne({ _id: annotation._id }, {
-            target: {
-              id: target,
-              state: {
-                id: contentId,
-              },
-            },
-          })
-          updatedCount += 1
-        }
+      const mapping = await Mapping.findOne({ uri: target })
+      const contentId = mapping && (mapping.identifier || []).find(id => id.startsWith("urn:jskos:mapping:content:"))
+      const update = contentId ? {
+        target: {
+          id: target,
+          state: {
+            id: contentId,
+          },
+        },
+      } : {
+        target: { id: target },
       }
+      await Annotation.updateOne({ _id: annotation._id }, update)
+      updatedCount += 1
     }
     console.log(`... done (${updatedCount} annotations updated).`)