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

cherry-pick(#7065): Have annotations work with domain objects that ha… #7078

Merged
merged 1 commit into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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