Fix multi user notebook (#5563)

* Detect remote changes to notebook object and re-render entries. Detect changes to tags as well

* Do not throw an error when getCurrentUser is called, just return undefined. Code needs a way of testing whether there is a valid user

* Support remote sync of annotations for notebook entries

* Fix bug in notebook spec that prevented multi-user notebook regression being detected

* Fixes edge case where an annotation does not initially exist

* Use structuredClone instead of JSON functions. Fix logical error in entry modification attribution. Address magical value

Co-authored-by: Scott Bell <scott@traclabs.com>
This commit is contained in:
Andrew Henry 2022-07-29 02:14:09 -07:00 committed by GitHub
parent 8dc8a1c0a9
commit 60f20c64d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 86 additions and 33 deletions

View File

@ -88,7 +88,7 @@ export default class ObjectAPI {
this.cache = {};
this.interceptorRegistry = new InterceptorRegistry();
this.SYNCHRONIZED_OBJECT_TYPES = ['notebook', 'plan'];
this.SYNCHRONIZED_OBJECT_TYPES = ['notebook', 'plan', 'annotation'];
this.errors = {
Conflict: ConflictError

View File

@ -83,9 +83,11 @@ class UserAPI extends EventEmitter {
* @throws Will throw an error if no user provider is set
*/
getCurrentUser() {
this.noProviderCheck();
return this._provider.getCurrentUser();
if (!this.hasProvider()) {
return Promise.resolve(undefined);
} else {
return this._provider.getCurrentUser();
}
}
/**

View File

@ -296,12 +296,17 @@ export default {
window.addEventListener('orientationchange', this.formatSidebar);
window.addEventListener('hashchange', this.setSectionAndPageFromUrl);
this.filterAndSortEntries();
this.unobserveEntries = this.openmct.objects.observe(this.domainObject, '*', this.filterAndSortEntries);
},
beforeDestroy() {
if (this.unlisten) {
this.unlisten();
}
if (this.unobserveEntries) {
this.unobserveEntries();
}
window.removeEventListener('orientationchange', this.formatSidebar);
window.removeEventListener('hashchange', this.setSectionAndPageFromUrl);
},

View File

@ -88,6 +88,7 @@
:annotation-type="openmct.annotation.ANNOTATION_TYPES.NOTEBOOK"
:annotation-search-type="openmct.objects.SEARCH_TYPES.NOTEBOOK_ANNOTATIONS"
:target-specific-details="{entryId: entry.id}"
@tags-updated="timestampAndUpdate"
/>
<div class="c-snapshots c-ne__embeds">
@ -146,6 +147,8 @@ import { saveNotebookImageDomainObject, updateNamespaceOfDomainObject } from '..
import Moment from 'moment';
const UNKNOWN_USER = 'Unknown';
export default {
components: {
NotebookEmbed,
@ -206,7 +209,8 @@ export default {
return {
targetKeyString,
entryId: this.entry.id
entryId: this.entry.id,
modified: this.entry.modified
};
},
createdOnTime() {
@ -283,7 +287,7 @@ export default {
await this.addNewEmbed(objectPath);
}
this.$emit('updateEntry', this.entry);
this.timestampAndUpdate();
},
findPositionInArray(array, id) {
let position = -1;
@ -321,7 +325,7 @@ export default {
// TODO: remove notebook snapshot object using object remove API
this.entry.embeds.splice(embedPosition, 1);
this.$emit('updateEntry', this.entry);
this.timestampAndUpdate();
},
updateEmbed(newEmbed) {
this.entry.embeds.some(e => {
@ -333,6 +337,17 @@ export default {
return found;
});
this.timestampAndUpdate();
},
async timestampAndUpdate() {
const user = await this.openmct.user.getCurrentUser();
if (user === undefined) {
this.entry.modifiedBy = UNKNOWN_USER;
}
this.entry.modified = Date.now();
this.$emit('updateEntry', this.entry);
},
editingEntry() {
@ -342,7 +357,7 @@ export default {
const value = $event.target.innerText;
if (value !== this.entry.text && value.match(/\S/)) {
this.entry.text = value;
this.$emit('updateEntry', this.entry);
this.timestampAndUpdate();
} else {
this.$emit('cancelEdit');
}

View File

@ -211,10 +211,17 @@ describe("Notebook plugin:", () => {
describe("synchronization", () => {
let objectCloneToSyncFrom;
beforeEach(() => {
objectCloneToSyncFrom = structuredClone(notebookViewObject);
objectCloneToSyncFrom.persisted = notebookViewObject.modified + 1;
});
it("updates an entry when another user modifies it", () => {
expect(getEntryText(0).innerText).toBe("First Test Entry");
notebookViewObject.configuration.entries["test-section-1"]["test-page-1"][0].text = "Modified entry text";
objectProviderObserver(notebookViewObject);
objectCloneToSyncFrom.configuration.entries["test-section-1"]["test-page-1"][0].text = "Modified entry text";
objectProviderObserver(objectCloneToSyncFrom);
return Vue.nextTick().then(() => {
expect(getEntryText(0).innerText).toBe("Modified entry text");
@ -223,13 +230,13 @@ describe("Notebook plugin:", () => {
it("shows new entry when another user adds one", () => {
expect(allNotebookEntryElements().length).toBe(2);
notebookViewObject.configuration.entries["test-section-1"]["test-page-1"].push({
objectCloneToSyncFrom.configuration.entries["test-section-1"]["test-page-1"].push({
"id": "entry-3",
"createdOn": 0,
"text": "Third Test Entry",
"embeds": []
});
objectProviderObserver(notebookViewObject);
objectProviderObserver(objectCloneToSyncFrom);
return Vue.nextTick().then(() => {
expect(allNotebookEntryElements().length).toBe(3);
@ -237,9 +244,9 @@ describe("Notebook plugin:", () => {
});
it("removes an entry when another user removes one", () => {
expect(allNotebookEntryElements().length).toBe(2);
let entries = notebookViewObject.configuration.entries["test-section-1"]["test-page-1"];
notebookViewObject.configuration.entries["test-section-1"]["test-page-1"] = entries.splice(0, 1);
objectProviderObserver(notebookViewObject);
let entries = objectCloneToSyncFrom.configuration.entries["test-section-1"]["test-page-1"];
objectCloneToSyncFrom.configuration.entries["test-section-1"]["test-page-1"] = entries.splice(0, 1);
objectProviderObserver(objectCloneToSyncFrom);
return Vue.nextTick().then(() => {
expect(allNotebookEntryElements().length).toBe(1);
@ -256,8 +263,8 @@ describe("Notebook plugin:", () => {
};
expect(allNotebookPageElements().length).toBe(2);
notebookViewObject.configuration.sections[0].pages.push(newPage);
objectProviderObserver(notebookViewObject);
objectCloneToSyncFrom.configuration.sections[0].pages.push(newPage);
objectProviderObserver(objectCloneToSyncFrom);
return Vue.nextTick().then(() => {
expect(allNotebookPageElements().length).toBe(3);
@ -267,8 +274,8 @@ describe("Notebook plugin:", () => {
it("updates the notebook when a user removes a page", () => {
expect(allNotebookPageElements().length).toBe(2);
notebookViewObject.configuration.sections[0].pages.splice(0, 1);
objectProviderObserver(notebookViewObject);
objectCloneToSyncFrom.configuration.sections[0].pages.splice(0, 1);
objectProviderObserver(objectCloneToSyncFrom);
return Vue.nextTick().then(() => {
expect(allNotebookPageElements().length).toBe(1);
@ -291,8 +298,8 @@ describe("Notebook plugin:", () => {
};
expect(allNotebookSectionElements().length).toBe(2);
notebookViewObject.configuration.sections.push(newSection);
objectProviderObserver(notebookViewObject);
objectCloneToSyncFrom.configuration.sections.push(newSection);
objectProviderObserver(objectCloneToSyncFrom);
return Vue.nextTick().then(() => {
expect(allNotebookSectionElements().length).toBe(3);
@ -301,8 +308,8 @@ describe("Notebook plugin:", () => {
it("updates the notebook when a user removes a section", () => {
expect(allNotebookSectionElements().length).toBe(2);
notebookViewObject.configuration.sections.splice(0, 1);
objectProviderObserver(notebookViewObject);
objectCloneToSyncFrom.configuration.sections.splice(0, 1);
objectProviderObserver(objectCloneToSyncFrom);
return Vue.nextTick().then(() => {
expect(allNotebookSectionElements().length).toBe(1);

View File

@ -287,7 +287,7 @@ class CouchObjectProvider {
this.objectQueue[key] = new CouchObjectQueue(undefined, response[REV]);
}
if (isNotebookType(object)) {
if (isNotebookType(object) || object.type === 'annotation') {
//Temporary measure until object sync is supported for all object types
//Always update notebook revision number because we have realtime sync, so always assume it's the latest.
this.objectQueue[key].updateRevision(response[REV]);

View File

@ -97,14 +97,17 @@ export default {
this.tagsChanged(this.annotation.tags);
},
deep: true
},
annotationQuery: {
handler() {
this.unloadAnnotation();
this.loadAnnotation();
},
deep: true
}
},
async mounted() {
this.annotation = await this.openmct.annotation.getAnnotation(this.annotationQuery, this.annotationSearchType);
this.addAnnotationListener(this.annotation);
if (this.annotation && this.annotation.tags) {
this.tagsChanged(this.annotation.tags);
}
mounted() {
this.loadAnnotation();
},
destroyed() {
if (this.removeTagsListener) {
@ -114,7 +117,23 @@ export default {
methods: {
addAnnotationListener(annotation) {
if (annotation && !this.removeTagsListener) {
this.removeTagsListener = this.openmct.objects.observe(annotation, 'tags', this.tagsChanged);
this.removeTagsListener = this.openmct.objects.observe(annotation, '*', (newAnnotation) => {
this.tagsChanged(newAnnotation.tags);
this.annotation = newAnnotation;
});
}
},
async loadAnnotation() {
this.annotation = await this.openmct.annotation.getAnnotation(this.annotationQuery, this.annotationSearchType);
this.addAnnotationListener(this.annotation);
if (this.annotation && this.annotation.tags) {
this.tagsChanged(this.annotation.tags);
}
},
unloadAnnotation() {
if (this.removeTagsListener) {
this.removeTagsListener();
this.removeTagsListener = undefined;
}
},
tagsChanged(newTags) {
@ -133,8 +152,11 @@ export default {
this.addedTags.push(newTagValue);
this.userAddingTag = true;
},
tagRemoved(tagToRemove) {
return this.openmct.annotation.removeAnnotationTag(this.annotation, tagToRemove);
async tagRemoved(tagToRemove) {
const result = await this.openmct.annotation.removeAnnotationTag(this.annotation, tagToRemove);
this.$emit('tags-updated');
return result;
},
async tagAdded(newTag) {
const annotationWasCreated = this.annotation === null || this.annotation === undefined;
@ -146,6 +168,8 @@ export default {
this.tagsChanged(this.annotation.tags);
this.userAddingTag = false;
this.$emit('tags-updated');
}
}
};