Skip to content

Commit

Permalink
cherry-pick(#7065): Have annotations work with domain objects that ha… (
Browse files Browse the repository at this point in the history
#7078)

cherry-pick(#7065): Have annotations work with domain objects that have dots

* 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
  • Loading branch information
ozyx committed Sep 21, 2023
1 parent af4ef8c commit d4e2716
Show file tree
Hide file tree
Showing 19 changed files with 257 additions and 140 deletions.
4 changes: 2 additions & 2 deletions e2e/tests/functional/plugins/plot/tagging.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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
Expand Down Expand Up @@ -284,7 +284,7 @@ test.describe('Plot Tagging', () => {
page,
canvas,
xEnd: 700,
yEnd: 215
yEnd: 240
});
await basicTagsTests(page);
await testTelemetryItem(page, alphaSineWave);
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/performance/tagging.perf.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ test.describe.fixme('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
Expand Down Expand Up @@ -265,7 +265,7 @@ test.describe.fixme('Plot Tagging Performance', () => {
page,
canvas,
xEnd: 700,
yEnd: 215
yEnd: 240
});
await basicTagsTests(page);
await testTelemetryItem(page, alphaSineWave);
Expand Down
33 changes: 20 additions & 13 deletions src/api/annotation/AnnotationAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 || [];
Expand All @@ -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
Expand All @@ -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`);
}

Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
33 changes: 18 additions & 15 deletions src/api/annotation/AnnotationAPISpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -166,7 +167,7 @@ describe('The Annotation API', () => {
tags: ['sometag'],
contentText: 'fooContext',
targetDomainObjects: [mockDomainObject],
targets: { fooTarget: {} }
targets: [{ keyString: 'fooTarget' }]
};
openmct.annotation.setNamespaceToSaveAnnotations('nameespaceThatDoesNotExist');
await openmct.annotation.create(annotationCreationArguments);
Expand All @@ -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);
Expand All @@ -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]
};
});
Expand Down Expand Up @@ -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', () => {
Expand Down
3 changes: 2 additions & 1 deletion src/api/objects/InMemorySearchProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] = [];
}
Expand Down
3 changes: 2 additions & 1 deletion src/api/objects/InMemorySearchWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] = [];
}
Expand Down
40 changes: 21 additions & 19 deletions src/plugins/imagery/components/AnnotationsCanvas.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
};
},
Expand Down Expand Up @@ -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: []
});
},
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
11 changes: 4 additions & 7 deletions src/plugins/inspectorViews/annotations/tags/TagEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ export default {
default: null
},
targets: {
type: Object,
type: Array,
required: true,
default: null
},
targetDomainObjects: {
type: Object,
type: Array,
required: true,
default: null
},
Expand Down Expand Up @@ -200,11 +200,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,
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/notebook/actions/ExportNotebookAsTextAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,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)) {
Expand Down

0 comments on commit d4e2716

Please sign in to comment.