[editor] Remove mako variables for editor id and notebooks as well as…

… the old notebooks listing page (#3657) In this commit I'm removing the editor_id and notebooks that gets set from the backend when opening the editor. The editor ID is now taken from the url parameter and notebooks isn't actually used anymore. As part of this I'm also removing the notebooks listing page which should be considered dead code as it's no longer reachable from the UI or via URL. This is part of the effort of removing inline script and our goal of moving towards CSR.
cloudera · Mar 19, 2024 · 1f30a3a · 1f30a3a
1 parent 1291cc8
commit 1f30a3a
Show file tree

Hide file tree

Showing 15 changed files with 86 additions and 382 deletions.
diff --git a/desktop/core/src/desktop/js/apps/editor/EditorViewModel.js b/desktop/core/src/desktop/js/apps/editor/EditorViewModel.js
@@ -35,13 +35,11 @@ import changeURLParameter from 'utils/url/changeURLParameter';
 import getParameter from 'utils/url/getParameter';
 
 export default class EditorViewModel {
-  constructor(editorId, notebooks, options, CoordinatorEditorViewModel, RunningCoordinatorModel) {
+  constructor(options, CoordinatorEditorViewModel, RunningCoordinatorModel) {
     // eslint-disable-next-line no-restricted-syntax
     console.log('Editor v2 enabled.');
 
-    this.editorId = editorId;
     this.snippetViewSettings = options.snippetViewSettings;
-    this.notebooks = notebooks;
 
     this.URLS = {
       editor: '/hue/editor',
@@ -313,14 +311,13 @@ export default class EditorViewModel {
   }
 
   async init() {
-    if (this.editorId) {
-      await this.openNotebook(this.editorId);
+    const editorId = getParameter('editor');
+    if (editorId) {
+      await this.openNotebook(editorId);
     } else if (getParameter('gist') !== '' || getParameter('type') !== '') {
       await this.newNotebook(getParameter('type'));
     } else if (getParameter('editor') !== '') {
       await this.openNotebook(getParameter('editor'));
-    } else if (this.notebooks.length > 0) {
-      this.loadNotebook(this.notebooks[0]); // Old way of loading json for /browse
     } else {
       await this.newNotebook();
     }

diff --git a/desktop/core/src/desktop/js/apps/editor/EditorViewModel.test.js b/desktop/core/src/desktop/js/apps/editor/EditorViewModel.test.js
@@ -0,0 +1,30 @@
+// Licensed to Cloudera, Inc. under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  Cloudera, Inc. licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import EditorViewModel from './EditorViewModel';
+import changeURLParameter from 'utils/url/changeURLParameter';
+
+describe('EditorViewModel.js', () => {
+  it('should load the document if opened with an ID in the "editor" url parameter', async () => {
+    changeURLParameter('editor', '123');
+    const vm = new EditorViewModel({});
+    const spy = jest.spyOn(vm, 'openNotebook').mockImplementation(() => Promise.resolve());
+
+    await vm.init();
+
+    expect(spy).toHaveBeenCalledWith('123');
+  });
+});
diff --git a/desktop/core/src/desktop/js/apps/editor/app.js b/desktop/core/src/desktop/js/apps/editor/app.js
@@ -303,18 +303,12 @@ huePubSub.subscribe('app.dom.loaded', app => {
 
     if (window.EDITOR_ENABLE_QUERY_SCHEDULING) {
       viewModel = new EditorViewModel(
-        window.EDITOR_ID,
-        window.NOTEBOOKS_JSON,
         window.EDITOR_VIEW_MODEL_OPTIONS,
         window.CoordinatorEditorViewModel,
         window.RunningCoordinatorModel
       );
     } else {
-      viewModel = new EditorViewModel(
-        window.EDITOR_ID,
-        window.NOTEBOOKS_JSON,
-        window.EDITOR_VIEW_MODEL_OPTIONS
-      );
+      viewModel = new EditorViewModel(window.EDITOR_VIEW_MODEL_OPTIONS);
     }
     ko.applyBindings(viewModel, $(window.EDITOR_BINDABLE_ELEMENT)[0]);
     viewModel.init();

diff --git a/desktop/core/src/desktop/js/apps/notebook/NotebookViewModel.js b/desktop/core/src/desktop/js/apps/notebook/NotebookViewModel.js
@@ -37,7 +37,7 @@ import getParameter from 'utils/url/getParameter';
 import UUID from 'utils/string/UUID';
 
 export default class NotebookViewModel {
-  constructor(editor_id, notebooks, options, CoordinatorEditorViewModel, RunningCoordinatorModel) {
+  constructor(options, CoordinatorEditorViewModel, RunningCoordinatorModel) {
     const self = this;
 
     self.URLS = {
@@ -450,14 +450,13 @@ export default class NotebookViewModel {
     };
 
     self.init = function () {
-      if (editor_id) {
-        self.openNotebook(editor_id);
+      const editorId = options?.editorId || getParameter('editor');
+      if (editorId) {
+        self.openNotebook(editorId);
       } else if (getParameter('gist') !== '') {
         self.newNotebook(getParameter('type'));
       } else if (getParameter('editor') !== '') {
         self.openNotebook(getParameter('editor'));
-      } else if (notebooks.length > 0) {
-        self.loadNotebook(notebooks[0]); // Old way of loading json for /browse
       } else if (getParameter('type') !== '') {
         self.newNotebook(getParameter('type'));
       } else {

diff --git a/desktop/core/src/desktop/js/apps/notebook/NotebookViewModel.test.js b/desktop/core/src/desktop/js/apps/notebook/NotebookViewModel.test.js
@@ -0,0 +1,39 @@
+// Licensed to Cloudera, Inc. under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  Cloudera, Inc. licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import NotebookViewModel from './NotebookViewModel';
+import changeURLParameter from 'utils/url/changeURLParameter';
+
+describe('NotebookViewModel.js', () => {
+  it('should load the document if opened with an ID in the "editor" url parameter', async () => {
+    changeURLParameter('editor', '123');
+    const vm = new NotebookViewModel({});
+    const spy = jest.spyOn(vm, 'openNotebook').mockImplementation(() => Promise.resolve());
+
+    vm.init();
+
+    expect(spy).toHaveBeenCalledWith('123');
+  });
+
+  it('should load the document if opened with an "editorId" option', async () => {
+    const vm = new NotebookViewModel({ editorId: '234' });
+    const spy = jest.spyOn(vm, 'openNotebook').mockImplementation(() => Promise.resolve());
+
+    vm.init();
+
+    expect(spy).toHaveBeenCalledWith('234');
+  });
+});
diff --git a/desktop/core/src/desktop/js/apps/notebook/app.js b/desktop/core/src/desktop/js/apps/notebook/app.js
@@ -542,18 +542,12 @@ huePubSub.subscribe('app.dom.loaded', app => {
 
     if (window.EDITOR_ENABLE_QUERY_SCHEDULING) {
       viewModel = new NotebookViewModel(
-        window.EDITOR_ID,
-        window.NOTEBOOKS_JSON,
         window.EDITOR_VIEW_MODEL_OPTIONS,
         window.CoordinatorEditorViewModel,
         window.RunningCoordinatorModel
       );
     } else {
-      viewModel = new NotebookViewModel(
-        window.EDITOR_ID,
-        window.NOTEBOOKS_JSON,
-        window.EDITOR_VIEW_MODEL_OPTIONS
-      );
+      viewModel = new NotebookViewModel(window.EDITOR_VIEW_MODEL_OPTIONS);
     }
     ko.applyBindings(viewModel, $(window.EDITOR_BINDABLE_ELEMENT)[0]);
     viewModel.init();

diff --git a/desktop/core/src/desktop/js/ko/components/ko.historyPanel.js b/desktop/core/src/desktop/js/ko/components/ko.historyPanel.js
@@ -169,7 +169,7 @@ class HistoryPanel {
       self.historyPanelVisible(false);
     });
 
-    self.editorViewModel = new NotebookViewModel(null, '', {
+    self.editorViewModel = new NotebookViewModel({
       user: window.LOGGED_USERNAME,
       userId: window.LOGGED_USER_ID,
       languages: [

diff --git a/desktop/libs/indexer/src/indexer/templates/importer.mako b/desktop/libs/indexer/src/indexer/templates/importer.mako
@@ -3027,7 +3027,8 @@ ${ commonheader(_("Importer"), "indexer", user, request, "60px") | n,unicode }
             self.jobId(resp.handle.id);
             $('#importerNotebook').html($('#importerNotebook-progress').html());
 
-            self.editorVM = new window.NotebookViewModel(resp.history_uuid, '', {
+            self.editorVM = new window.NotebookViewModel({
+              editorId: resp.history_uuid,
               user: '${ user.username }',
               userId: ${ user.id },
               languages: [{name: "Java", type: "java"}, {name: "Hive SQL", type: "hive"}], // TODO reuse

diff --git a/desktop/libs/indexer/src/indexer/templates/indexer.mako b/desktop/libs/indexer/src/indexer/templates/indexer.mako
@@ -846,7 +846,8 @@ ${ commonheader(_("Solr Indexes"), "search", user, request, "60px") | n,unicode
           self.editorId(resp.history_id);
           self.jobId(resp.handle.id);
           $('#notebook').html($('#notebook-progress').html());
-          self.editorVM = new window.NotebookViewModel(resp.history_uuid, '', {
+          self.editorVM = new window.NotebookViewModel({
+            editorId: resp.history_uuid,
             user: '${ user.username }',
             userId: ${ user.id },
             languages: [{name: "Java SQL", type: "java"}],

diff --git a/desktop/libs/notebook/src/notebook/templates/editor2.mako b/desktop/libs/notebook/src/notebook/templates/editor2.mako
@@ -1289,10 +1289,6 @@ There is no bridge to KO for components using this integration. Example using in
 
   window.EDITOR_ENABLE_QUERY_SCHEDULING = '${ ENABLE_QUERY_SCHEDULING.get() }' === 'True';
 
-  window.EDITOR_ID = ${ editor_id or 'null' };
-
-  window.NOTEBOOKS_JSON = ${ notebooks_json | n,unicode };
-
   window.SQL_ANALYZER_AUTO_UPLOAD_QUERIES = '${ OPTIMIZER.AUTO_UPLOAD_QUERIES.get() }' === 'True';
 
   window.SQL_ANALYZER_AUTO_UPLOAD_DDL = '${ OPTIMIZER.AUTO_UPLOAD_DDL.get() }' === 'True';

diff --git a/desktop/libs/notebook/src/notebook/templates/editor_components.mako b/desktop/libs/notebook/src/notebook/templates/editor_components.mako
@@ -2260,10 +2260,6 @@ ${ sqlSyntaxDropdown.sqlSyntaxDropdown() }
 
   window.EDITOR_ENABLE_QUERY_SCHEDULING = '${ ENABLE_QUERY_SCHEDULING.get() }' === 'True';
 
-  window.EDITOR_ID = ${ editor_id or 'null' };
-
-  window.NOTEBOOKS_JSON = ${ notebooks_json | n,unicode };
-
   window.SQL_ANALYZER_AUTO_UPLOAD_QUERIES = '${ OPTIMIZER.AUTO_UPLOAD_QUERIES.get() }' === 'True';
 
   window.SQL_ANALYZER_AUTO_UPLOAD_DDL = '${ OPTIMIZER.AUTO_UPLOAD_DDL.get() }' === 'True';

diff --git a/desktop/libs/notebook/src/notebook/templates/editor_m.mako b/desktop/libs/notebook/src/notebook/templates/editor_m.mako
@@ -173,6 +173,7 @@ ${ commonheader_m(editor_type, editor_type, user, request, "68px") | n,unicode }
   ace.config.set("basePath", "${ static('desktop/js/ace') }");
 
   var VIEW_MODEL_OPTIONS = $.extend(${ options_json | n,unicode }, {
+    editorId: ${ editor_id or 'null' },
     user: '${ user.username }',
     userId: ${ user.id },
     assistAvailable: true,
@@ -305,7 +306,7 @@ ${ commonheader_m(editor_type, editor_type, user, request, "68px") | n,unicode }
       }
     }
 
-    viewModel = new window.NotebookViewModel(${ editor_id or 'null' }, ${ notebooks_json | n,unicode }, VIEW_MODEL_OPTIONS);
+    viewModel = new window.NotebookViewModel(VIEW_MODEL_OPTIONS);
     ko.applyBindings(viewModel);
     viewModel.init();
   });