Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have annotations work with domain objects that have dots #7065

Merged
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 @@ -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
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('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('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 @@
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 @@
* @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 @@
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`);

Check warning on line 149 in src/api/annotation/AnnotationAPI.js

View check run for this annotation

Codecov / codecov/patch

src/api/annotation/AnnotationAPI.js#L149

Added line #L149 was not covered by tests
}

if (!targetDomainObjects.length) {
throw new Error(`At least one targetDomainObject is required to create an annotation`);
}

Expand Down Expand Up @@ -181,7 +185,7 @@
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 @@
}

#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 @@
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 @@
#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('namespaceThatDoesNotExist');
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 @@
// 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;

Check warning on line 77 in src/plugins/imagery/components/AnnotationsCanvas.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/imagery/components/AnnotationsCanvas.vue#L76-L77

Added lines #L76 - L77 were not covered by tests
const annotationRectangleForPixelDepth =
this.transformRectangleToPixelDense(annotationRectangle);
const indexNumber = builtAnnotationsIndex.add(
Expand Down Expand Up @@ -141,20 +145,17 @@
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));

Check warning on line 151 in src/plugins/imagery/components/AnnotationsCanvas.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/imagery/components/AnnotationsCanvas.vue#L151

Added line #L151 was not covered by tests
});
});
this.selectedAnnotations = annotations;
this.drawAnnotations();

return {
targetDomainObjects,
targetDomainObjects: [this.domainObject],
targetDetails
};
},
Expand Down Expand Up @@ -292,23 +293,23 @@
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 = [
{

Check warning on line 304 in src/plugins/imagery/components/AnnotationsCanvas.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/imagery/components/AnnotationsCanvas.vue#L303-L304

Added lines #L303 - L304 were not covered by tests
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 @@
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 @@
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;

Check warning on line 100 in src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue#L100

Added line #L100 was not covered by tests

if (showingTagsEditor) {
return true;
Expand All @@ -106,7 +106,7 @@
return false;
},
targetDomainObjects() {
return this?.selection?.[0]?.[0]?.context?.targetDomainObjects ?? {};
return this?.selection?.[0]?.[0]?.context?.targetDomainObjects ?? [];

Check warning on line 109 in src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue#L109

Added line #L109 was not covered by tests
},
selectedAnnotations() {
return this?.selection?.[0]?.[0]?.context?.annotations;
Expand Down Expand Up @@ -167,9 +167,8 @@
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;

Check warning on line 171 in src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/inspectorViews/annotations/AnnotationsInspectorView.vue#L171

Added line #L171 was not covered by tests
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 @@ -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
},
Expand Down Expand Up @@ -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,
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 @@ -33,7 +33,9 @@
getTagsForEntry(entry, domainObjectKeyString, annotations) {
const foundTags = [];
annotations.forEach((annotation) => {
const target = annotation.targets?.[domainObjectKeyString];
const target = annotation.targets.find(
(annotationTarget) => annotationTarget.keyString === domainObjectKeyString

Check warning on line 37 in src/plugins/notebook/actions/ExportNotebookAsTextAction.js

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/actions/ExportNotebookAsTextAction.js#L36-L37

Added lines #L36 - L37 were not covered by tests
);
if (target?.entryId === entry.id) {
annotation.tags.forEach((tag) => {
if (!foundTags.includes(tag)) {
Expand Down