Merge branch 'master' into user-attribution

Merge'n

e2e/appActions.js

@@ -225,15 +225,14 @@ async function getHashUrlToDomainObject(page, uuid) {
 }
 
 /**
- * Utilizes the OpenMCT API to detect if the given object has an active transaction (is in Edit mode).
+ * Utilizes the OpenMCT API to detect if the UI is in Edit mode.
  * @private
  * @param {import('@playwright/test').Page} page
- * @param {string | import('../src/api/objects/ObjectAPI').Identifier} identifier
- * @return {Promise<boolean>} true if the object has an active transaction, false otherwise
+ * @return {Promise<boolean>} true if the Open MCT is in Edit Mode
  */
 async function _isInEditMode(page, identifier) {
     // eslint-disable-next-line no-return-await
-    return await page.evaluate((objectIdentifier) => window.openmct.objects.isTransactionActive(objectIdentifier), identifier);
+    return await page.evaluate(() => window.openmct.editor.isEditing());
 }
 
 /**

src/api/annotation/AnnotationAPISpec.js

@@ -94,7 +94,6 @@ describe("The Annotation API", () => {
         openmct.startHeadless();
     });
     afterEach(async () => {
-        openmct.objects.providers = {};
         await resetApplicationState(openmct);
     });
     it("is defined", () => {

src/api/objects/ObjectAPI.js

@@ -96,7 +96,7 @@ export default class ObjectAPI {
         this.cache = {};
         this.interceptorRegistry = new InterceptorRegistry();
 
-        this.SYNCHRONIZED_OBJECT_TYPES = ['notebook', 'plan', 'annotation'];
+        this.SYNCHRONIZED_OBJECT_TYPES = ['notebook', 'restricted-notebook', 'plan', 'annotation'];
 
         this.errors = {
             Conflict: ConflictError
@@ -204,13 +204,13 @@ export default class ObjectAPI {
         }
 
         identifier = utils.parseKeyString(identifier);
-        let dirtyObject;
-        if (this.isTransactionActive()) {
-            dirtyObject = this.transaction.getDirtyObject(identifier);
-        }
 
-        if (dirtyObject) {
-            return Promise.resolve(dirtyObject);
+        if (this.isTransactionActive()) {
+            let dirtyObject = this.transaction.getDirtyObject(identifier);
+
+            if (dirtyObject) {
+                return Promise.resolve(dirtyObject);
+            }
         }
 
         const provider = this.getProvider(identifier);
@@ -354,10 +354,8 @@ export default class ObjectAPI {
      * @returns {Promise} a promise which will resolve when the domain object
      *          has been saved, or be rejected if it cannot be saved
      */
-    save(domainObject) {
-        let provider = this.getProvider(domainObject.identifier);
-        let savedResolve;
-        let savedReject;
+    async save(domainObject) {
+        const provider = this.getProvider(domainObject.identifier);
         let result;
 
         if (!this.isPersistable(domainObject.identifier)) {
@@ -366,27 +364,37 @@ export default class ObjectAPI {
             result = Promise.resolve(true);
         } else {
             const persistedTime = Date.now();
-            if (domainObject.persisted === undefined) {
-                result = new Promise((resolve, reject) => {
-                    savedResolve = resolve;
-                    savedReject = reject;
-                });
-                domainObject.persisted = persistedTime;
-                const newObjectPromise = provider.create(domainObject);
-                if (newObjectPromise) {
-                    newObjectPromise.then(response => {
-                        this.mutate(domainObject, 'persisted', persistedTime);
-                        savedResolve(response);
-                    }).catch((error) => {
-                        savedReject(error);
-                    });
-                } else {
-                    result = Promise.reject(`[ObjectAPI][save] Object provider returned ${newObjectPromise} when creating new object.`);
-                }
+            const username = await this.#getCurrentUsername();
+            const isNewObject = domainObject.persisted === undefined;
+            let savedResolve;
+            let savedReject;
+            let savedObjectPromise;
+
+            result = new Promise((resolve, reject) => {
+                savedResolve = resolve;
+                savedReject = reject;
+            });
+
+            this.#mutate(domainObject, 'persisted', persistedTime);
+            this.#mutate(domainObject, 'modifiedBy', username);
+
+            if (isNewObject) {
+                this.#mutate(domainObject, 'created', persistedTime);
+                this.#mutate(domainObject, 'createdBy', username);
+
+                savedObjectPromise = provider.create(domainObject);
             } else {
-                domainObject.persisted = persistedTime;
-                this.mutate(domainObject, 'persisted', persistedTime);
-                result = provider.update(domainObject);
+                savedObjectPromise = provider.update(domainObject);
+            }
+
+            if (savedObjectPromise) {
+                savedObjectPromise.then(response => {
+                    savedResolve(response);
+                }).catch((error) => {
+                    savedReject(error);
+                });
+            } else {
+                result = Promise.reject(`[ObjectAPI][save] Object provider returned ${savedObjectPromise} when ${isNewObject ? 'creating new' : 'updating'} object.`);
             }
         }
 
@@ -399,8 +407,21 @@ export default class ObjectAPI {
         });
     }
 
+    async #getCurrentUsername() {
+        const user = await this.openmct.user.getCurrentUser();
+        let username;
+
+        if (user !== undefined) {
+            username = user.getName();
+        }
+
+        return username;
+    }
+
     /**
      * After entering into edit mode, creates a new instance of Transaction to keep track of changes in Objects
+     *
+     * @returns {Transaction} a new Transaction that was just created
      */
     startTransaction() {
         if (this.isTransactionActive()) {
@@ -408,6 +429,8 @@ export default class ObjectAPI {
         }
 
         this.transaction = new Transaction(this);
+
+        return this.transaction;
     }
 
     /**
@@ -480,14 +503,16 @@ export default class ObjectAPI {
     }
 
     /**
-     * Modify a domain object.
+     * Modify a domain object. Internal to ObjectAPI, won't call save after.
+     * @private
+     *
      * @param {module:openmct.DomainObject} object the object to mutate
      * @param {string} path the property to modify
      * @param {*} value the new value for this property
      * @method mutate
      * @memberof module:openmct.ObjectAPI#
      */
-    mutate(domainObject, path, value) {
+    #mutate(domainObject, path, value) {
         if (!this.supportsMutation(domainObject.identifier)) {
             throw `Error: Attempted to mutate immutable object ${domainObject.name}`;
         }
@@ -508,6 +533,18 @@ export default class ObjectAPI {
             //Destroy temporary mutable object
             this.destroyMutable(mutableDomainObject);
         }
+    }
+
+    /**
+     * Modify a domain object and save.
+     * @param {module:openmct.DomainObject} object the object to mutate
+     * @param {string} path the property to modify
+     * @param {*} value the new value for this property
+     * @method mutate
+     * @memberof module:openmct.ObjectAPI#
+     */
+    mutate(domainObject, path, value) {
+        this.#mutate(domainObject, path, value);
 
         if (this.isTransactionActive()) {
             this.transaction.add(domainObject);
@@ -684,7 +721,7 @@ export default class ObjectAPI {
     }
 
     isTransactionActive() {
-        return Boolean(this.transaction && this.openmct.editor.isEditing());
+        return this.transaction !== undefined && this.transaction !== null;
     }
 
     #hasAlreadyBeenPersisted(domainObject) {

src/api/objects/ObjectAPISpec.js

@@ -8,13 +8,27 @@ describe("The Object API", () => {
     let mockDomainObject;
     const TEST_NAMESPACE = "test-namespace";
     const TEST_KEY = "test-key";
+    const USERNAME = 'Joan Q Public';
     const FIFTEEN_MINUTES = 15 * 60 * 1000;
 
     beforeEach((done) => {
         typeRegistry = jasmine.createSpyObj('typeRegistry', [
             'get'
         ]);
+        const userProvider = {
+            isLoggedIn() {
+                return true;
+            },
+            getCurrentUser() {
+                return Promise.resolve({
+                    getName() {
+                        return USERNAME;
+                    }
+                });
+            }
+        };
         openmct = createOpenMct();
+        openmct.user.setProvider(userProvider);
         objectAPI = openmct.objects;
 
         openmct.editor = {};
@@ -63,19 +77,34 @@ describe("The Object API", () => {
                 mockProvider.update.and.returnValue(Promise.resolve(true));
                 objectAPI.addProvider(TEST_NAMESPACE, mockProvider);
             });
-            it("Calls 'create' on provider if object is new", () => {
-                objectAPI.save(mockDomainObject);
+            it("Adds a 'created' timestamp to new objects", async () => {
+                await objectAPI.save(mockDomainObject);
+                expect(mockDomainObject.created).not.toBeUndefined();
+            });
+            it("Calls 'create' on provider if object is new", async () => {
+                await objectAPI.save(mockDomainObject);
                 expect(mockProvider.create).toHaveBeenCalled();
                 expect(mockProvider.update).not.toHaveBeenCalled();
             });
-            it("Calls 'update' on provider if object is not new", () => {
+            it("Calls 'update' on provider if object is not new", async () => {
                 mockDomainObject.persisted = Date.now() - FIFTEEN_MINUTES;
                 mockDomainObject.modified = Date.now();
 
-                objectAPI.save(mockDomainObject);
+                await objectAPI.save(mockDomainObject);
                 expect(mockProvider.create).not.toHaveBeenCalled();
                 expect(mockProvider.update).toHaveBeenCalled();
             });
+            it("Sets the current user for 'createdBy' on new objects", async () => {
+                await objectAPI.save(mockDomainObject);
+                expect(mockDomainObject.createdBy).toBe(USERNAME);
+            });
+            it("Sets the current user for 'modifedBy' on existing objects", async () => {
+                mockDomainObject.persisted = Date.now() - FIFTEEN_MINUTES;
+                mockDomainObject.modified = Date.now();
+
+                await objectAPI.save(mockDomainObject);
+                expect(mockDomainObject.modifiedBy).toBe(USERNAME);
+            });
 
             it("Does not persist if the object is unchanged", () => {
                 mockDomainObject.persisted =

src/plugins/plan/pluginSpec.js

@@ -264,7 +264,7 @@ describe('the plugin', function () {
         it('provides an inspector view with the version information if available', () => {
             componentObject = component.$root.$children[0];
             const propertiesEls = componentObject.$el.querySelectorAll('.c-inspect-properties__row');
-            expect(propertiesEls.length).toEqual(4);
+            expect(propertiesEls.length).toEqual(6);
             const found = Array.from(propertiesEls).some((propertyEl) => {
                 return (propertyEl.children[0].innerHTML.trim() === 'Version'
                     && propertyEl.children[1].innerHTML.trim() === 'v1');

src/plugins/remove/RemoveAction.js

@@ -19,8 +19,12 @@
  * this source code distribution or the Licensing information page available
  * at runtime from the About dialog for additional information.
  *****************************************************************************/
+
 export default class RemoveAction {
+    #transaction;
+
     constructor(openmct) {
+
         this.name = 'Remove';
         this.key = 'remove';
         this.description = 'Remove this object from its containing object.';
@@ -29,17 +33,25 @@ export default class RemoveAction {
         this.priority = 1;
 
         this.openmct = openmct;
+
+        this.removeFromComposition = this.removeFromComposition.bind(this); // for access to private transaction variable
     }
 
-    invoke(objectPath) {
+    async invoke(objectPath) {
         let object = objectPath[0];
         let parent = objectPath[1];
-        this.showConfirmDialog(object).then(() => {
-            this.removeFromComposition(parent, object);
-            if (this.inNavigationPath(object)) {
-                this.navigateTo(objectPath.slice(1));
-            }
-        }).catch(() => {});
+
+        try {
+            await this.showConfirmDialog(object);
+        } catch (error) {
+            return; // form canceled, exit invoke
+        }
+
+        await this.removeFromComposition(parent, object);
+
+        if (this.inNavigationPath(object)) {
+            this.navigateTo(objectPath.slice(1));
+        }
     }
 
     showConfirmDialog(object) {
@@ -81,20 +93,21 @@ export default class RemoveAction {
         this.openmct.router.navigate('#/browse/' + urlPath);
     }
 
-    removeFromComposition(parent, child) {
-        let composition = parent.composition.filter(id =>
-            !this.openmct.objects.areIdsEqual(id, child.identifier)
-        );
+    async removeFromComposition(parent, child) {
+        this.startTransaction();
 
-        this.openmct.objects.mutate(parent, 'composition', composition);
+        const composition = this.openmct.composition.get(parent);
+        composition.remove(child);
+
+        if (!this.isAlias(child, parent)) {
+            this.openmct.objects.mutate(child, 'location', null);
+        }
 
         if (this.inNavigationPath(child) && this.openmct.editor.isEditing()) {
             this.openmct.editor.save();
         }
 
-        if (!this.isAlias(child, parent)) {
-            this.openmct.objects.mutate(child, 'location', null);
-        }
+        await this.saveTransaction();
     }
 
     isAlias(child, parent) {
@@ -132,4 +145,23 @@ export default class RemoveAction {
             && parentType.definition.creatable
             && Array.isArray(parent.composition);
     }
+
+    startTransaction() {
+        if (!this.openmct.objects.isTransactionActive()) {
+            this.#transaction = this.openmct.objects.startTransaction();
+        }
+    }
+
+    saveTransaction() {
+        if (!this.#transaction) {
+            return;
+        }
+
+        return this.#transaction.commit()
+            .catch(error => {
+                throw error;
+            }).finally(() => {
+                this.openmct.objects.endTransaction();
+            });
+    }
 }

src/ui/inspector/InspectorDetailsSpec.js

@@ -38,6 +38,8 @@ describe('the inspector', () => {
         folderItem = {
             name: 'folder',
             type: 'folder',
+            createdBy: 'John Q',
+            modifiedBy: 'Public',
             id: 'mock-folder-key',
             identifier: {
                 namespace: '',
@@ -74,6 +76,8 @@ describe('the inspector', () => {
         const [
             title,
             type,
+            createdBy,
+            modifiedBy,
             notes,
             timestamp
         ] = details;
@@ -87,6 +91,14 @@ describe('the inspector', () => {
             .toEqual('Type');
         expect(type.value.toLowerCase())
             .toEqual(folderItem.type);
+        expect(createdBy.name)
+            .toEqual('Created By');
+        expect(createdBy.value)
+            .toEqual(folderItem.createdBy);
+        expect(modifiedBy.name)
+            .toEqual('Modified By');
+        expect(modifiedBy.value)
+            .toEqual(folderItem.modifiedBy);
         expect(notes.value)
             .toEqual('This object should have some notes');
 

src/ui/inspector/details/Properties.vue

@@ -90,10 +90,13 @@ export default {
                 return;
             }
 
+            const UNKNOWN_USER = 'Unknown';
             const title = this.domainObject.name;
             const typeName = this.type ? this.type.definition.name : `Unknown: ${this.domainObject.type}`;
-            const timestampLabel = this.domainObject.modified ? 'Modified' : 'Created';
-            const timestamp = this.domainObject.modified ? this.domainObject.modified : this.domainObject.created;
+            const createdTimestamp = this.domainObject.created;
+            const createdBy = this.domainObject.createdBy ? this.domainObject.createdBy : UNKNOWN_USER;
+            const modifiedBy = this.domainObject.modifiedBy ? this.domainObject.modifiedBy : UNKNOWN_USER;
+            const modifiedTimestamp = this.domainObject.modified ? this.domainObject.modified : this.domainObject.created;
             const notes = this.domainObject.notes;
             const version = this.domainObject.version;
 
@@ -105,6 +108,14 @@ export default {
                 {
                     name: 'Type',
                     value: typeName
+                },
+                {
+                    name: 'Created By',
+                    value: createdBy
+                },
+                {
+                    name: 'Modified By',
+                    value: modifiedBy
                 }
             ];
 
@@ -115,15 +126,28 @@ export default {
                 });
             }
 
-            if (timestamp !== undefined) {
-                const formattedTimestamp = Moment.utc(timestamp)
+            if (createdTimestamp !== undefined) {
+                const formattedCreatedTimestamp = Moment.utc(createdTimestamp)
                     .format('YYYY-MM-DD[\n]HH:mm:ss')
                     + ' UTC';
 
                 details.push(
                     {
-                        name: timestampLabel,
-                        value: formattedTimestamp
+                        name: 'Created',
+                        value: formattedCreatedTimestamp
+                    }
+                );
+            }
+
+            if (modifiedTimestamp !== undefined) {
+                const formattedModifiedTimestamp = Moment.utc(modifiedTimestamp)
+                    .format('YYYY-MM-DD[\n]HH:mm:ss')
+                    + ' UTC';
+
+                details.push(
+                    {
+                        name: 'Modified',
+                        value: formattedModifiedTimestamp
                     }
                 );
             }