Have annotations work with domain objects that have dots (#7065)

* migrating to new structure - wip * notebooks work, now to plots and images * resolve conflicts * fix search * add to readme * spelling * fix unit test * add search by view for big search speedup * spelling * fix out of order search * improve reliability of plot tagging tests
nasa · Sep 21, 2023 · 482f1f6 · 482f1f6
1 parent 05c7a81
commit 482f1f6
Show file tree

Hide file tree

Showing 19 changed files with 257 additions and 140 deletions.
diff --git a/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js b/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js
@@ -41,7 +41,7 @@ test.describe('Plot Tagging', () => {
    * @param {Number} yEnd a telemetry item with a plot
    * @returns {Promise}
    */
-  async function createTags({ page, canvas, xEnd = 700, yEnd = 480 }) {
+  async function createTags({ page, canvas, xEnd = 700, yEnd = 520 }) {
     await canvas.hover({ trial: true });
 
     //Alt+Shift Drag Start to select some points to tag
@@ -284,7 +284,7 @@ test.describe('Plot Tagging', () => {
       page,
       canvas,
       xEnd: 700,
-      yEnd: 215
+      yEnd: 240
     });
     await basicTagsTests(page);
     await testTelemetryItem(page, alphaSineWave);

diff --git a/e2e/tests/performance/tagging.perf.spec.js b/e2e/tests/performance/tagging.perf.spec.js
@@ -41,7 +41,7 @@ test.describe('Plot Tagging Performance', () => {
    * @param {Number} yEnd a telemetry item with a plot
    * @returns {Promise}
    */
-  async function createTags({ page, canvas, xEnd = 700, yEnd = 480 }) {
+  async function createTags({ page, canvas, xEnd = 700, yEnd = 520 }) {
     await canvas.hover({ trial: true });
 
     //Alt+Shift Drag Start to select some points to tag
@@ -265,7 +265,7 @@ test.describe('Plot Tagging Performance', () => {
       page,
       canvas,
       xEnd: 700,
-      yEnd: 215
+      yEnd: 240
     });
     await basicTagsTests(page);
     await testTelemetryItem(page, alphaSineWave);

diff --git a/src/api/annotation/AnnotationAPI.js b/src/api/annotation/AnnotationAPI.js
@@ -100,7 +100,7 @@ export default class AnnotationAPI extends EventEmitter {
       creatable: false,
       cssClass: 'icon-notebook',
       initialize: function (domainObject) {
-        domainObject.targets = domainObject.targets || {};
+        domainObject.targets = domainObject.targets || [];
         domainObject._deleted = domainObject._deleted || false;
         domainObject.originalContextPath = domainObject.originalContextPath || '';
         domainObject.tags = domainObject.tags || [];
@@ -117,10 +117,10 @@ export default class AnnotationAPI extends EventEmitter {
    * @property {ANNOTATION_TYPES} annotationType the type of annotation to create (e.g., PLOT_SPATIAL)
    * @property {Tag[]} tags tags to add to the annotation, e.g., SCIENCE for science related annotations
    * @property {String} contentText Some text to add to the annotation, e.g. ("This annotation is about science")
-   * @property {Object<string, Object>} targets The targets ID keystrings and their specific properties.
-   * For plots, this will be a bounding box, e.g.: {maxY: 100, minY: 0, maxX: 100, minX: 0}
+   * @property {Array<Object>} targets The targets ID keystrings and their specific properties.
+   * For plots, this will be a bounding box, e.g.: {keyString: "d8385009-789d-457b-acc7-d50ba2fd55ea", maxY: 100, minY: 0, maxX: 100, minX: 0}
    * For notebooks, this will be an entry ID, e.g.: {entryId: "entry-ecb158f5-d23c-45e1-a704-649b382622ba"}
-   * @property {DomainObject>} targetDomainObjects the targets ID keystrings and the domain objects this annotation points to (e.g., telemetry objects for a plot)
+   * @property {DomainObject>[]} targetDomainObjects the domain objects this annotation points to (e.g., telemetry objects for a plot)
    */
   /**
    * @method create
@@ -141,11 +141,15 @@ export default class AnnotationAPI extends EventEmitter {
       throw new Error(`Unknown annotation type: ${annotationType}`);
     }
 
-    if (!Object.keys(targets).length) {
+    if (!targets.length) {
       throw new Error(`At least one target is required to create an annotation`);
     }
 
-    if (!Object.keys(targetDomainObjects).length) {
+    if (targets.some((target) => !target.keyString)) {
+      throw new Error(`All targets require a keyString to create an annotation`);
+    }
+
+    if (!targetDomainObjects.length) {
       throw new Error(`At least one targetDomainObject is required to create an annotation`);
     }
 
@@ -181,7 +185,7 @@ export default class AnnotationAPI extends EventEmitter {
     const success = await this.openmct.objects.save(createdObject);
     if (success) {
       this.emit('annotationCreated', createdObject);
-      Object.values(targetDomainObjects).forEach((targetDomainObject) => {
+      targetDomainObjects.forEach((targetDomainObject) => {
         this.#updateAnnotationModified(targetDomainObject);
       });
 
@@ -321,7 +325,10 @@ export default class AnnotationAPI extends EventEmitter {
   }
 
   #addTagMetaInformationToTags(tags) {
-    return tags.map((tagKey) => {
+    // Convert to Set and back to Array to remove duplicates
+    const uniqueTags = [...new Set(tags)];
+
+    return uniqueTags.map((tagKey) => {
       const tagModel = this.availableTags[tagKey];
       tagModel.tagID = tagKey;
 
@@ -363,7 +370,8 @@ export default class AnnotationAPI extends EventEmitter {
     const modelAddedToResults = await Promise.all(
       results.map(async (result) => {
         const targetModels = await Promise.all(
-          Object.keys(result.targets).map(async (targetID) => {
+          result.targets.map(async (target) => {
+            const targetID = target.keyString;
             const targetModel = await this.openmct.objects.get(targetID);
             const targetKeyString = this.openmct.objects.makeKeyString(targetModel.identifier);
             const originalPathObjects = await this.openmct.objects.getOriginalPath(targetKeyString);
@@ -410,13 +418,12 @@ export default class AnnotationAPI extends EventEmitter {
   #breakApartSeparateTargets(results) {
     const separateResults = [];
     results.forEach((result) => {
-      Object.keys(result.targets).forEach((targetID) => {
+      result.targets.forEach((target) => {
+        const targetID = target.keyString;
         const separatedResult = {
           ...result
         };
-        separatedResult.targets = {
-          [targetID]: result.targets[targetID]
-        };
+        separatedResult.targets = [target];
         separatedResult.targetModels = result.targetModels.filter((targetModel) => {
           const targetKeyString = this.openmct.objects.makeKeyString(targetModel.identifier);
 

diff --git a/src/api/annotation/AnnotationAPISpec.js b/src/api/annotation/AnnotationAPISpec.js
@@ -62,11 +62,12 @@ describe('The Annotation API', () => {
         key: 'anAnnotationKey',
         namespace: 'fooNameSpace'
       },
-      targets: {
-        'fooNameSpace:some-object': {
+      targets: [
+        {
+          keyString: 'fooNameSpace:some-object',
           entryId: 'fooBarEntry'
         }
-      }
+      ]
     };
 
     mockObjectProvider = jasmine.createSpyObj('mock provider', ['create', 'update', 'get']);
@@ -121,7 +122,7 @@ describe('The Annotation API', () => {
         tags: ['sometag'],
         contentText: 'fooContext',
         targetDomainObjects: [mockDomainObject],
-        targets: { fooTarget: {} }
+        targets: [{ keyString: 'fooTarget' }]
       };
       const annotationObject = await openmct.annotation.create(annotationCreationArguments);
       expect(annotationObject).toBeDefined();
@@ -136,7 +137,7 @@ describe('The Annotation API', () => {
         tags: ['sometag'],
         contentText: 'fooContext',
         targetDomainObjects: [mockDomainObject],
-        targets: { fooTarget: {} }
+        targets: [{ keyString: 'fooTarget' }]
       };
       openmct.annotation.setNamespaceToSaveAnnotations('fooNameSpace');
       const annotationObject = await openmct.annotation.create(annotationCreationArguments);
@@ -166,7 +167,7 @@ describe('The Annotation API', () => {
           tags: ['sometag'],
           contentText: 'fooContext',
           targetDomainObjects: [mockDomainObject],
-          targets: { fooTarget: {} }
+          targets: [{ keyString: 'fooTarget' }]
         };
         openmct.annotation.setNamespaceToSaveAnnotations('namespaceThatDoesNotExist');
         await openmct.annotation.create(annotationCreationArguments);
@@ -183,7 +184,7 @@ describe('The Annotation API', () => {
           tags: ['sometag'],
           contentText: 'fooContext',
           targetDomainObjects: [mockDomainObject],
-          targets: { fooTarget: {} }
+          targets: [{ keyString: 'fooTarget' }]
         };
         openmct.annotation.setNamespaceToSaveAnnotations('immutableProvider');
         await openmct.annotation.create(annotationCreationArguments);
@@ -202,7 +203,7 @@ describe('The Annotation API', () => {
         annotationType: openmct.annotation.ANNOTATION_TYPES.NOTEBOOK,
         tags: ['aWonderfulTag'],
         contentText: 'fooContext',
-        targets: { 'fooNameSpace:some-object': { entryId: 'fooBarEntry' } },
+        targets: [{ keyString: 'fooNameSpace:some-object', entryId: 'fooBarEntry' }],
         targetDomainObjects: [mockDomainObject]
       };
     });
@@ -272,17 +273,19 @@ describe('The Annotation API', () => {
     let comparator;
 
     beforeEach(() => {
-      targets = {
-        fooTarget: {
+      targets = [
+        {
+          keyString: 'fooTarget',
           foo: 42
         }
-      };
-      otherTargets = {
-        fooTarget: {
+      ];
+      otherTargets = [
+        {
+          keyString: 'fooTarget',
           bar: 42
         }
-      };
-      comparator = (t1, t2) => t1.fooTarget.foo === t2.fooTarget.bar;
+      ];
+      comparator = (t1, t2) => t1[0].foo === t2[0].bar;
     });
 
     it('can add a comparator function', () => {

diff --git a/src/api/objects/InMemorySearchProvider.js b/src/api/objects/InMemorySearchProvider.js
@@ -435,7 +435,8 @@ class InMemorySearchProvider {
   }
 
   localIndexAnnotation(objectToIndex, model) {
-    Object.keys(model.targets).forEach((targetID) => {
+    model.targets.forEach((target) => {
+      const targetID = target.keyString;
       if (!this.localIndexedAnnotationsByDomainObject[targetID]) {
         this.localIndexedAnnotationsByDomainObject[targetID] = [];
       }

diff --git a/src/api/objects/InMemorySearchWorker.js b/src/api/objects/InMemorySearchWorker.js
@@ -57,7 +57,8 @@
   };
 
   function indexAnnotation(objectToIndex, model) {
-    Object.keys(model.targets).forEach((targetID) => {
+    model.targets.forEach((target) => {
+      const targetID = target.keyString;
       if (!indexedAnnotationsByDomainObject[targetID]) {
         indexedAnnotationsByDomainObject[targetID] = [];
       }

diff --git a/src/plugins/imagery/components/AnnotationsCanvas.vue b/src/plugins/imagery/components/AnnotationsCanvas.vue
@@ -33,6 +33,8 @@
 
 <script>
 import Flatbush from 'flatbush';
+import { toRaw } from 'vue';
+
 const EXISTING_ANNOTATION_STROKE_STYLE = '#D79078';
 const EXISTING_ANNOTATION_FILL_STYLE = 'rgba(202, 202, 142, 0.2)';
 const SELECTED_ANNOTATION_STROKE_COLOR = '#BD8ECC';
@@ -70,7 +72,9 @@ export default {
         // create a flatbush index for the annotations
         const builtAnnotationsIndex = new Flatbush(this.imageryAnnotations.length);
         this.imageryAnnotations.forEach((annotation) => {
-          const annotationRectangle = annotation.targets[this.keyString].rectangle;
+          const annotationRectangle = annotation.targets.find(
+            (target) => target.keyString === this.keyString
+          )?.rectangle;
           const annotationRectangleForPixelDepth =
             this.transformRectangleToPixelDense(annotationRectangle);
           const indexNumber = builtAnnotationsIndex.add(
@@ -141,20 +145,17 @@ export default {
       this.prepareExistingAnnotationSelection(incomingSelectedAnnotations);
     },
     prepareExistingAnnotationSelection(annotations) {
-      const targetDomainObjects = {};
-      targetDomainObjects[this.keyString] = this.domainObject;
-
-      const targetDetails = {};
+      const targetDetails = [];
       annotations.forEach((annotation) => {
-        Object.entries(annotation.targets).forEach(([key, value]) => {
-          targetDetails[key] = value;
+        annotation.targets.forEach((target) => {
+          targetDetails.push(toRaw(target));
         });
       });
       this.selectedAnnotations = annotations;
       this.drawAnnotations();
 
       return {
-        targetDomainObjects,
+        targetDomainObjects: [this.domainObject],
         targetDetails
       };
     },
@@ -292,23 +293,23 @@ export default {
       this.dragging = false;
       this.selectedAnnotations = [];
 
-      const targetDomainObjects = {};
-      targetDomainObjects[this.keyString] = this.domainObject;
-      const targetDetails = {};
       const rectangleFromCanvas = {
         x: this.newAnnotationRectangle.x,
         y: this.newAnnotationRectangle.y,
         width: this.newAnnotationRectangle.width,
         height: this.newAnnotationRectangle.height
       };
       const rectangleWithoutPixelScale = this.transformRectangleFromPixelDense(rectangleFromCanvas);
-      targetDetails[this.keyString] = {
-        rectangle: rectangleWithoutPixelScale,
-        time: this.image.time
-      };
+      const targetDetails = [
+        {
+          rectangle: rectangleWithoutPixelScale,
+          time: this.image.time,
+          keyString: this.keyString
+        }
+      ];
       this.selectImageAnnotations({
         targetDetails,
-        targetDomainObjects,
+        targetDomainObjects: [this.domainObject],
         annotations: []
       });
     },
@@ -403,9 +404,10 @@ export default {
         if (annotation._deleted) {
           return;
         }
-        const rectangleForPixelDensity = this.transformRectangleToPixelDense(
-          annotation.targets[this.keyString].rectangle
-        );
+        const annotationRectangle = annotation.targets.find(
+          (target) => target.keyString === this.keyString
+        )?.rectangle;
+        const rectangleForPixelDensity = this.transformRectangleToPixelDense(annotationRectangle);
         if (this.isSelectedAnnotation(annotation)) {
           this.drawRectInCanvas(
             rectangleForPixelDensity,

diff --git a/src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue b/src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue
@@ -94,10 +94,10 @@ export default {
       return this?.selection?.[0]?.[0]?.context?.item;
     },
     targetDetails() {
-      return this?.selection?.[0]?.[0]?.context?.targetDetails ?? {};
+      return this?.selection?.[0]?.[0]?.context?.targetDetails ?? [];
     },
     shouldShowTagsEditor() {
-      const showingTagsEditor = Object.keys(this.targetDetails).length > 0;
+      const showingTagsEditor = this.targetDetails?.length > 0;
 
       if (showingTagsEditor) {
         return true;
@@ -106,7 +106,7 @@ export default {
       return false;
     },
     targetDomainObjects() {
-      return this?.selection?.[0]?.[0]?.context?.targetDomainObjects ?? {};
+      return this?.selection?.[0]?.[0]?.context?.targetDomainObjects ?? [];
     },
     selectedAnnotations() {
       return this?.selection?.[0]?.[0]?.context?.annotations;
@@ -167,9 +167,8 @@ export default {
       this.unobserveEntries = {};
 
       this.selection = selection;
-      const targetKeys = Object.keys(this.targetDomainObjects);
-      targetKeys.forEach((targetKey) => {
-        const targetObject = this.targetDomainObjects[targetKey];
+      this.targetDomainObjects.forEach((targetObject) => {
+        const targetKey = targetObject.keyString;
         this.lastLocalAnnotationCreations[targetKey] = targetObject?.annotationLastCreated ?? 0;
         if (!this.unobserveEntries[targetKey]) {
           this.unobserveEntries[targetKey] = this.openmct.objects.observe(

diff --git a/src/plugins/inspectorViews/annotations/tags/TagEditor.vue b/src/plugins/inspectorViews/annotations/tags/TagEditor.vue
@@ -69,12 +69,12 @@ export default {
       default: null
     },
     targets: {
-      type: Object,
+      type: Array,
       required: true,
       default: null
     },
     targetDomainObjects: {
-      type: Object,
+      type: Array,
       required: true,
       default: null
     },
@@ -201,11 +201,8 @@ export default {
         const contentText = `${this.annotationType} tag`;
 
         // need to get raw version of target domain objects for comparisons to work
-        const rawTargetDomainObjects = {};
-        Object.keys(this.targetDomainObjects).forEach((targetDomainObjectKey) => {
-          rawTargetDomainObjects[targetDomainObjectKey] = toRaw(
-            this.targetDomainObjects[targetDomainObjectKey]
-          );
+        const rawTargetDomainObjects = this.targetDomainObjects.map((targetDomainObject) => {
+          return toRaw(targetDomainObject);
         });
         const annotationCreationArguments = {
           name: contentText,

diff --git a/src/plugins/notebook/actions/ExportNotebookAsTextAction.js b/src/plugins/notebook/actions/ExportNotebookAsTextAction.js
@@ -33,7 +33,9 @@ export default class ExportNotebookAsTextAction {
   getTagsForEntry(entry, domainObjectKeyString, annotations) {
     const foundTags = [];
     annotations.forEach((annotation) => {
-      const target = annotation.targets?.[domainObjectKeyString];
+      const target = annotation.targets.find(
+        (annotationTarget) => annotationTarget.keyString === domainObjectKeyString
+      );
       if (target?.entryId === entry.id) {
         annotation.tags.forEach((tag) => {
           if (!foundTags.includes(tag)) {