diff --git a/e2e/appActions.js b/e2e/appActions.js index 3064ecf079..7b84080bbb 100644 --- a/e2e/appActions.js +++ b/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} true if the object has an active transaction, false otherwise + * @return {Promise} 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()); } /** diff --git a/src/api/annotation/AnnotationAPISpec.js b/src/api/annotation/AnnotationAPISpec.js index cc362d8a18..b8464a35d7 100644 --- a/src/api/annotation/AnnotationAPISpec.js +++ b/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", () => { diff --git a/src/api/objects/ObjectAPI.js b/src/api/objects/ObjectAPI.js index 1d0ccfe25c..5c22765f91 100644 --- a/src/api/objects/ObjectAPI.js +++ b/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) { diff --git a/src/api/objects/ObjectAPISpec.js b/src/api/objects/ObjectAPISpec.js index 950b356be0..c15b3c7db5 100644 --- a/src/api/objects/ObjectAPISpec.js +++ b/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 = diff --git a/src/plugins/plan/pluginSpec.js b/src/plugins/plan/pluginSpec.js index d981ac2653..781ae7710f 100644 --- a/src/plugins/plan/pluginSpec.js +++ b/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'); diff --git a/src/plugins/remove/RemoveAction.js b/src/plugins/remove/RemoveAction.js index 8a0b9ec4ca..fd6700bde9 100644 --- a/src/plugins/remove/RemoveAction.js +++ b/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(); + }); + } } diff --git a/src/ui/inspector/InspectorDetailsSpec.js b/src/ui/inspector/InspectorDetailsSpec.js index 24553c5c4d..208069fb08 100644 --- a/src/ui/inspector/InspectorDetailsSpec.js +++ b/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'); diff --git a/src/ui/inspector/details/Properties.vue b/src/ui/inspector/details/Properties.vue index fe0edda9e0..d14a2b4ed9 100644 --- a/src/ui/inspector/details/Properties.vue +++ b/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 } ); }