From d232dacc6522ac0641710006e09f4462334a6719 Mon Sep 17 00:00:00 2001 From: Nikhil Date: Thu, 19 Nov 2020 08:38:14 -0800 Subject: [PATCH] [Notebook] new entries on brand new notebook not rendered (#3496) * [Notebook] new entries on brand new notebook not rendered #3488 * Refactored code to 'mutateObject' from one place only, add page to newly created section immediately, update entries copy then call mutate to update it inside domainObject. Co-authored-by: Jamie V Co-authored-by: Andrew Henry --- src/plugins/notebook/components/Notebook.vue | 67 +++++++------ .../notebook/components/PageCollection.vue | 6 +- .../notebook/components/SectionCollection.vue | 4 - src/plugins/notebook/components/Sidebar.vue | 94 ++++++++++++------- .../notebook/utils/notebook-entries.js | 76 +++++++-------- .../notebook/utils/notebook-entriesSpec.js | 42 +++------ src/ui/components/ObjectLabel.vue | 4 +- src/ui/layout/BrowseBar.vue | 5 +- 8 files changed, 147 insertions(+), 151 deletions(-) diff --git a/src/plugins/notebook/components/Notebook.vue b/src/plugins/notebook/components/Notebook.vue index ff2ae94ef4..345d4828c3 100644 --- a/src/plugins/notebook/components/Notebook.vue +++ b/src/plugins/notebook/components/Notebook.vue @@ -24,12 +24,12 @@ :default-section-id="defaultSectionId" :domain-object="internalDomainObject" :page-title="internalDomainObject.configuration.pageTitle" - :pages="pages" :section-title="internalDomainObject.configuration.sectionTitle" :sections="sections" + :selected-section="selectedSection" :sidebar-covers-entries="sidebarCoversEntries" - @updatePage="updatePage" - @updateSection="updateSection" + @pagesChanged="pagesChanged" + @sectionsChanged="sectionsChanged" @toggleNav="toggleNav" />
@@ -111,7 +111,7 @@ import Search from '@/ui/components/search.vue'; import SearchResults from './SearchResults.vue'; import Sidebar from './Sidebar.vue'; import { clearDefaultNotebook, getDefaultNotebook, setDefaultNotebook, setDefaultNotebookSection, setDefaultNotebookPage } from '../utils/notebook-storage'; -import { DEFAULT_CLASS, addNotebookEntry, createNewEmbed, getNotebookEntries } from '../utils/notebook-entries'; +import { DEFAULT_CLASS, addNotebookEntry, createNewEmbed, getNotebookEntries, mutateObject } from '../utils/notebook-entries'; import objectUtils from 'objectUtils'; import { throttle } from 'lodash'; @@ -220,7 +220,7 @@ export default { return s; }); - this.updateSection({ sections }); + this.sectionsChanged({ sections }); this.throttledSearchItem(''); }, createNotebookStorageObject() { @@ -309,7 +309,7 @@ export default { return null; } - return this.openmct.objects.get(oldNotebookStorage.notebookMeta.identifier).then(d => d); + return this.openmct.objects.get(oldNotebookStorage.notebookMeta.identifier); }, getPage(section, id) { return section.pages.find(p => p.id === id); @@ -379,9 +379,6 @@ export default { return this.sections.find(section => section.isSelected); }, - mutateObject(key, value) { - this.openmct.objects.mutate(this.internalDomainObject, key, value); - }, navigateToSectionPage() { const { pageId, sectionId } = this.openmct.router.getParams(); if (!pageId || !sectionId) { @@ -398,7 +395,7 @@ export default { return s; }); - this.updateSection({ sections }); + this.sectionsChanged({ sections }); }, newEntry(embed = null) { this.search = ''; @@ -411,6 +408,24 @@ export default { orientationChange() { this.formatSidebar(); }, + pagesChanged({ pages = [], id = null}) { + const selectedSection = this.getSelectedSection(); + if (!selectedSection) { + return; + } + + selectedSection.pages = pages; + const sections = this.sections.map(section => { + if (section.id === selectedSection.id) { + section = selectedSection; + } + + return section; + }); + + this.sectionsChanged({ sections }); + this.updateDefaultNotebookPage(pages, id); + }, removeDefaultClass(domainObject) { if (!domainObject) { return; @@ -423,7 +438,7 @@ export default { } classList.splice(index, 1); - this.openmct.objects.mutate(domainObject, 'classList', classList); + mutateObject(this.openmct, domainObject, 'classList', classList); }, searchItem(input) { this.search = input; @@ -440,12 +455,14 @@ export default { setDefaultNotebook(this.openmct, notebookStorage); } - if (this.defaultSectionId.length === 0 || this.defaultSectionId !== notebookStorage.section.id) { + if (this.defaultSectionId && this.defaultSectionId.length === 0 || this.defaultSectionId !== notebookStorage.section.id) { this.defaultSectionId = notebookStorage.section.id; + setDefaultNotebookSection(notebookStorage.section); } - if (this.defaultPageId.length === 0 || this.defaultPageId !== notebookStorage.page.id) { + if (this.defaultPageId && this.defaultPageId.length === 0 || this.defaultPageId !== notebookStorage.page.id) { this.defaultPageId = notebookStorage.page.id; + setDefaultNotebookPage(notebookStorage.page); } }, updateDefaultNotebookPage(pages, id) { @@ -509,29 +526,11 @@ export default { const notebookEntries = configuration.entries || {}; notebookEntries[this.selectedSection.id][this.selectedPage.id] = entries; - this.mutateObject('configuration.entries', notebookEntries); + mutateObject(this.openmct, this.internalDomainObject, 'configuration.entries', notebookEntries); }, updateInternalDomainObject(domainObject) { this.internalDomainObject = domainObject; }, - updatePage({ pages = [], id = null}) { - const selectedSection = this.getSelectedSection(); - if (!selectedSection) { - return; - } - - selectedSection.pages = pages; - const sections = this.sections.map(section => { - if (section.id === selectedSection.id) { - section = selectedSection; - } - - return section; - }); - - this.updateSection({ sections }); - this.updateDefaultNotebookPage(pages, id); - }, updateParams(sections) { const selectedSection = sections.find(s => s.isSelected); if (!selectedSection) { @@ -555,8 +554,8 @@ export default { pageId }); }, - updateSection({ sections, id = null }) { - this.mutateObject('configuration.sections', sections); + sectionsChanged({ sections, id = null }) { + mutateObject(this.openmct, this.internalDomainObject, 'configuration.sections', sections); this.updateParams(sections); this.updateDefaultNotebookSection(sections, id); diff --git a/src/plugins/notebook/components/PageCollection.vue b/src/plugins/notebook/components/PageCollection.vue index f3f605c338..4131531a61 100644 --- a/src/plugins/notebook/components/PageCollection.vue +++ b/src/plugins/notebook/components/PageCollection.vue @@ -66,14 +66,10 @@ export default { } } }, - data() { - return { - }; - }, methods: { deletePage(id) { const selectedSection = this.sections.find(s => s.isSelected); - const page = this.pages.filter(p => p.id !== id); + const page = this.pages.find(p => p.id !== id); deleteNotebookEntries(this.openmct, this.domainObject, selectedSection, page); const selectedPage = this.pages.find(p => p.isSelected); diff --git a/src/plugins/notebook/components/SectionCollection.vue b/src/plugins/notebook/components/SectionCollection.vue index d6bed6ea7a..e3981087a8 100644 --- a/src/plugins/notebook/components/SectionCollection.vue +++ b/src/plugins/notebook/components/SectionCollection.vue @@ -53,10 +53,6 @@ export default { } } }, - data() { - return { - }; - }, methods: { deleteSection(id) { const section = this.sections.find(s => s.id === id); diff --git a/src/plugins/notebook/components/Sidebar.vue b/src/plugins/notebook/components/Sidebar.vue index a3ed1d18a6..c12e56475e 100644 --- a/src/plugins/notebook/components/Sidebar.vue +++ b/src/plugins/notebook/components/Sidebar.vue @@ -18,7 +18,7 @@ :domain-object="domainObject" :sections="sections" :section-title="sectionTitle" - @updateSection="updateSection" + @updateSection="sectionsChanged" />
@@ -48,7 +48,7 @@ :sidebar-covers-entries="sidebarCoversEntries" :page-title="pageTitle" @toggleNav="toggleNav" - @updatePage="updatePage" + @updatePage="pagesChanged" /> @@ -85,13 +85,6 @@ export default { return {}; } }, - pages: { - type: Array, - required: true, - default() { - return []; - } - }, pageTitle: { type: String, default() { @@ -122,9 +115,16 @@ export default { return { }; }, + computed: { + pages() { + const selectedSection = this.sections.find(section => section.isSelected); + + return selectedSection && selectedSection.pages || []; + } + }, watch: { - pages(newpages) { - if (!newpages.length) { + pages(newPages) { + if (!newPages.length) { this.addPage(); } }, @@ -141,55 +141,79 @@ export default { }, methods: { addPage() { + const newPage = this.createNewPage(); + const pages = this.addNewPage(newPage); + + this.pagesChanged({ + pages, + id: newPage.id + }); + }, + addSection() { + const newSection = this.createNewSection(); + const sections = this.addNewSection(newSection); + + this.sectionsChanged({ + sections, + id: newSection.id + }); + }, + addNewPage(page) { + const pages = this.pages.map(p => { + p.isSelected = false; + + return p; + }); + + return pages.concat(page); + }, + addNewSection(section) { + const sections = this.sections.map(s => { + s.isSelected = false; + + return s; + }); + + return sections.concat(section); + }, + createNewPage() { const pageTitle = this.pageTitle; const id = uuid(); - const page = { + + return { id, isDefault: false, isSelected: true, name: `Unnamed ${pageTitle}`, pageTitle }; - - this.pages.forEach(p => p.isSelected = false); - const pages = this.pages.concat(page); - - this.updatePage({ - pages, - id - }); }, - addSection() { + createNewSection() { const sectionTitle = this.sectionTitle; const id = uuid(); - const section = { + const page = this.createNewPage(); + const pages = [page]; + + return { id, isDefault: false, isSelected: true, name: `Unnamed ${sectionTitle}`, - pages: [], + pages, sectionTitle }; - - this.sections.forEach(s => s.isSelected = false); - const sections = this.sections.concat(section); - - this.updateSection({ - sections, - id - }); }, toggleNav() { this.$emit('toggleNav'); }, - updatePage({ pages, id }) { - this.$emit('updatePage', { + pagesChanged({ pages, id }) { + this.$emit('pagesChanged', { pages, id }); }, - updateSection({ sections, id }) { - this.$emit('updateSection', { + sectionsChanged({ sections, id }) { + this.$emit('sectionsChanged', { sections, id }); diff --git a/src/plugins/notebook/utils/notebook-entries.js b/src/plugins/notebook/utils/notebook-entries.js index 8c606beb5b..0b8e342ec7 100644 --- a/src/plugins/notebook/utils/notebook-entries.js +++ b/src/plugins/notebook/utils/notebook-entries.js @@ -1,5 +1,4 @@ import objectLink from '../../../ui/mixins/object-link'; - export const DEFAULT_CLASS = 'is-notebook-default'; const TIME_BOUNDS = { START_BOUND: 'tc.startBound', @@ -8,6 +7,29 @@ const TIME_BOUNDS = { END_DELTA: 'tc.endDelta' }; +export function addEntryIntoPage(notebookStorage, entries, entry) { + const defaultSection = notebookStorage.section; + const defaultPage = notebookStorage.page; + if (!defaultSection || !defaultPage) { + return; + } + + const newEntries = JSON.parse(JSON.stringify(entries)); + let section = newEntries[defaultSection.id]; + if (!section) { + newEntries[defaultSection.id] = {}; + } + + let page = newEntries[defaultSection.id][defaultPage.id]; + if (!page) { + newEntries[defaultSection.id][defaultPage.id] = []; + } + + newEntries[defaultSection.id][defaultPage.id].push(entry); + + return newEntries; +} + export function getHistoricLinkInFixedMode(openmct, bounds, historicLink) { if (historicLink.includes('tc.mode=fixed')) { return historicLink; @@ -38,35 +60,6 @@ export function getHistoricLinkInFixedMode(openmct, bounds, historicLink) { return params.join('&'); } -export function getNotebookDefaultEntries(notebookStorage, domainObject) { - if (!notebookStorage || !domainObject) { - return null; - } - - const defaultSection = notebookStorage.section; - const defaultPage = notebookStorage.page; - if (!defaultSection || !defaultPage) { - return null; - } - - const configuration = domainObject.configuration; - const entries = configuration.entries || {}; - - let section = entries[defaultSection.id]; - if (!section) { - section = {}; - entries[defaultSection.id] = section; - } - - let page = entries[defaultSection.id][defaultPage.id]; - if (!page) { - page = []; - entries[defaultSection.id][defaultPage.id] = []; - } - - return entries[defaultSection.id][defaultPage.id]; -} - export function createNewEmbed(snapshotMeta, snapshot = '') { const { bounds, @@ -120,24 +113,25 @@ export function addNotebookEntry(openmct, domainObject, notebookStorage, embed = ? [embed] : []; - const defaultEntries = getNotebookDefaultEntries(notebookStorage, domainObject); const id = `entry-${date}`; - defaultEntries.push({ + const entry = { id, createdOn: date, text: '', embeds - }); + }; + + const newEntries = addEntryIntoPage(notebookStorage, entries, entry); addDefaultClass(domainObject); - openmct.objects.mutate(domainObject, 'configuration.entries', entries); + mutateObject(openmct, domainObject, 'configuration.entries', newEntries); return id; } export function getNotebookEntries(domainObject, selectedSection, selectedPage) { if (!domainObject || !selectedSection || !selectedPage) { - return null; + return; } const configuration = domainObject.configuration; @@ -145,12 +139,12 @@ export function getNotebookEntries(domainObject, selectedSection, selectedPage) let section = entries[selectedSection.id]; if (!section) { - return null; + return; } let page = entries[selectedSection.id][selectedPage.id]; if (!page) { - return null; + return; } return entries[selectedSection.id][selectedPage.id]; @@ -196,7 +190,11 @@ export function deleteNotebookEntries(openmct, domainObject, selectedSection, se delete entries[selectedSection.id][selectedPage.id]; - openmct.objects.mutate(domainObject, 'configuration.entries', entries); + mutateObject(openmct, domainObject, 'configuration.entries', entries); +} + +export function mutateObject(openmct, object, key, value) { + openmct.objects.mutate(object, key, value); } function addDefaultClass(domainObject) { @@ -206,4 +204,6 @@ function addDefaultClass(domainObject) { } classList.push(DEFAULT_CLASS); + + domainObject.classList = classList; } diff --git a/src/plugins/notebook/utils/notebook-entriesSpec.js b/src/plugins/notebook/utils/notebook-entriesSpec.js index 36d43275b9..4090311045 100644 --- a/src/plugins/notebook/utils/notebook-entriesSpec.js +++ b/src/plugins/notebook/utils/notebook-entriesSpec.js @@ -20,7 +20,7 @@ * at runtime from the About dialog for additional information. *****************************************************************************/ import * as NotebookEntries from './notebook-entries'; -import { createOpenMct, spyOnBuiltins, resetApplicationState } from 'utils/testing'; +import { createOpenMct, resetApplicationState } from 'utils/testing'; const notebookStorage = { domainObject: { @@ -121,7 +121,6 @@ describe('Notebook Entries:', () => { beforeEach(done => { openmct = createOpenMct(); window.localStorage.setItem('notebook-storage', null); - spyOnBuiltins(['mutate'], openmct.objects); done(); }); @@ -137,24 +136,16 @@ describe('Notebook Entries:', () => { expect(entries.length).toEqual(0); }); - it('addNotebookEntry mutates object', () => { + it('addNotebookEntry adds entry', (done) => { + const unlisten = openmct.objects.observe(notebookDomainObject, '*', (object) => { + const entries = NotebookEntries.getNotebookEntries(notebookDomainObject, selectedSection, selectedPage); + + expect(entries.length).toEqual(1); + done(); + unlisten(); + }); + NotebookEntries.addNotebookEntry(openmct, notebookDomainObject, notebookStorage); - - expect(openmct.objects.mutate).toHaveBeenCalled(); - }); - - it('addNotebookEntry adds entry', () => { - NotebookEntries.addNotebookEntry(openmct, notebookDomainObject, notebookStorage); - const entries = NotebookEntries.getNotebookEntries(notebookDomainObject, selectedSection, selectedPage); - - expect(entries.length).toEqual(1); - }); - - it('getEntryPosById returns valid position', () => { - const entryId = NotebookEntries.addNotebookEntry(openmct, notebookDomainObject, notebookStorage); - const position = NotebookEntries.getEntryPosById(entryId, notebookDomainObject, selectedSection, selectedPage); - - expect(position).toEqual(0); }); it('getEntryPosById returns valid position', () => { @@ -174,22 +165,13 @@ describe('Notebook Entries:', () => { expect(success).toBe(true); }); - it('deleteNotebookEntries mutates object', () => { - openmct.objects.mutate.calls.reset(); - - NotebookEntries.addNotebookEntry(openmct, notebookDomainObject, notebookStorage); - NotebookEntries.deleteNotebookEntries(openmct, notebookDomainObject, selectedSection, selectedPage); - - expect(openmct.objects.mutate).toHaveBeenCalledTimes(2); - }); - - it('deleteNotebookEntries deletes correct entry', () => { + it('deleteNotebookEntries deletes correct page entries', () => { NotebookEntries.addNotebookEntry(openmct, notebookDomainObject, notebookStorage); NotebookEntries.addNotebookEntry(openmct, notebookDomainObject, notebookStorage); NotebookEntries.deleteNotebookEntries(openmct, notebookDomainObject, selectedSection, selectedPage); const afterEntries = NotebookEntries.getNotebookEntries(notebookDomainObject, selectedSection, selectedPage); - expect(afterEntries).toEqual(null); + expect(afterEntries).toEqual(undefined); }); }); diff --git a/src/ui/components/ObjectLabel.vue b/src/ui/components/ObjectLabel.vue index 332e3d443d..de19484b45 100644 --- a/src/ui/components/ObjectLabel.vue +++ b/src/ui/components/ObjectLabel.vue @@ -18,7 +18,9 @@ title="This item is missing" > -
+
{{ observedObject.name }}
diff --git a/src/ui/layout/BrowseBar.vue b/src/ui/layout/BrowseBar.vue index ecb8175fab..55ca3c872b 100644 --- a/src/ui/layout/BrowseBar.vue +++ b/src/ui/layout/BrowseBar.vue @@ -9,10 +9,7 @@ >