From 1f30a3a4b20ffadf7b77f116b358cc340d0f7e3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20=C3=85hl=C3=A9n?= Date: Tue, 19 Mar 2024 16:08:49 +0100 Subject: [PATCH] [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. --- .../desktop/js/apps/editor/EditorViewModel.js | 11 +- .../js/apps/editor/EditorViewModel.test.js | 30 ++ .../core/src/desktop/js/apps/editor/app.js | 8 +- .../js/apps/notebook/NotebookViewModel.js | 9 +- .../apps/notebook/NotebookViewModel.test.js | 39 +++ .../core/src/desktop/js/apps/notebook/app.js | 8 +- .../js/ko/components/ko.historyPanel.js | 2 +- .../src/indexer/templates/importer.mako | 3 +- .../src/indexer/templates/indexer.mako | 3 +- .../src/notebook/templates/editor2.mako | 4 - .../notebook/templates/editor_components.mako | 4 - .../src/notebook/templates/editor_m.mako | 3 +- .../src/notebook/templates/notebooks.mako | 302 ------------------ desktop/libs/notebook/src/notebook/urls.py | 1 - desktop/libs/notebook/src/notebook/views.py | 41 --- 15 files changed, 86 insertions(+), 382 deletions(-) create mode 100644 desktop/core/src/desktop/js/apps/editor/EditorViewModel.test.js create mode 100644 desktop/core/src/desktop/js/apps/notebook/NotebookViewModel.test.js delete mode 100644 desktop/libs/notebook/src/notebook/templates/notebooks.mako diff --git a/desktop/core/src/desktop/js/apps/editor/EditorViewModel.js b/desktop/core/src/desktop/js/apps/editor/EditorViewModel.js index 9d0eb3de3e6..59aca1b3ea5 100644 --- 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 new file mode 100644 index 00000000000..5ee9120a450 --- /dev/null +++ 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 index 13001901250..917c535c5e5 100644 --- 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 index 03a128fe77f..5c6ba054eb3 100644 --- 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 new file mode 100644 index 00000000000..5791b4df1af --- /dev/null +++ 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 index 34d3e4c4f27..78d7377ca88 100644 --- 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 index 9fc79798f94..c6538eceacc 100644 --- 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 index 58ed709f961..bb83fe1a973 100644 --- 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 index e2fc4617282..221564be2a8 100644 --- 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 index f801aa1a28c..557e4b07ad6 100644 --- 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 index 5e23697d919..d785b5dba99 100644 --- 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 index e8d4d092579..6f495c17aa4 100644 --- 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(); }); diff --git a/desktop/libs/notebook/src/notebook/templates/notebooks.mako b/desktop/libs/notebook/src/notebook/templates/notebooks.mako deleted file mode 100644 index 0a8669e8bc1..00000000000 --- a/desktop/libs/notebook/src/notebook/templates/notebooks.mako +++ /dev/null @@ -1,302 +0,0 @@ -## 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 sys - from desktop.views import commonheader, commonfooter, commonimportexport, _ko - if sys.version_info[0] > 2: - from django.utils.translation import gettext as _ - else: - from django.utils.translation import ugettext as _ -%> -<%namespace name="actionbar" file="actionbar.mako" /> - -${ commonheader(_("Notebooks"), "spark", user, request, "60px") | n,unicode } - - -
-
-
-
- -
-
-
-
- - -
- -
-
- <%actionbar:render> - <%def name="search()"> - - - - <%def name="actions()"> -
- - ${ _('Copy') } - - - - ${ _('Delete') } - - - - - - ${ _('Export') } - -
- - - <%def name="creation()"> - % if editor_type != 'notebook': - - % else: - - % endif - ${ _('Create') } - - - ${ _('Import') } - - - - - - - - - - - - - - - - - - - - - - - - -
${ _('Name') }${ _('Description') }${ _('Owner') }${ _('Last Modified') }
-
- -
- -
-
- - -
- -
- - - - -
- -${ commonimportexport(request) | n,unicode } - - - - - - -${ commonfooter(request, messages) | n,unicode } diff --git a/desktop/libs/notebook/src/notebook/urls.py b/desktop/libs/notebook/src/notebook/urls.py index 9015ba3d607..c651695fbed 100644 --- a/desktop/libs/notebook/src/notebook/urls.py +++ b/desktop/libs/notebook/src/notebook/urls.py @@ -30,7 +30,6 @@ re_path(r'^$', notebook_views.notebook, name='index'), re_path(r'^notebook/?$', notebook_views.notebook, name='notebook'), re_path(r'^notebook_embeddable/?$', notebook_views.notebook_embeddable, name='notebook_embeddable'), - re_path(r'^notebooks/?$', notebook_views.notebooks, name='notebooks'), re_path(r'^new/?$', notebook_views.new, name='new'), re_path(r'^download/?$', notebook_views.download, name='download'), re_path(r'^install_examples/?$', notebook_views.install_examples, name='install_examples'), diff --git a/desktop/libs/notebook/src/notebook/views.py b/desktop/libs/notebook/src/notebook/views.py index c8f48a4e404..c2a20e9689b 100644 --- a/desktop/libs/notebook/src/notebook/views.py +++ b/desktop/libs/notebook/src/notebook/views.py @@ -15,15 +15,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -from builtins import object import json import logging import sys from django.urls import reverse -from django.db.models import Q from django.shortcuts import redirect -from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.http import require_POST from beeswax.data_export import DOWNLOAD_COOKIE_AGE @@ -34,7 +31,6 @@ from desktop.lib.connectors.models import Connector from desktop.lib.django_util import render, JsonResponse from desktop.lib.exceptions_renderable import PopupException -from desktop.lib.json_utils import JSONEncoderForHTML from desktop.models import Document2, Document, FilesystemException, _get_gist_document from desktop.views import serve_403_error from metadata.conf import has_optimizer, has_catalog, has_workload_analytics @@ -51,39 +47,8 @@ else: from django.utils.translation import ugettext as _ - LOG = logging.getLogger() - -def notebooks(request): - editor_type = request.GET.get('type', 'notebook') - - if editor_type != 'notebook': - if USE_NEW_EDITOR.get(): - notebooks = [doc.to_dict() for doc in Document2.objects.documents( - user=request.user).search_documents(types=['query-%s' % editor_type])] - else: - notebooks = [ - d.content_object.to_dict() - for d in Document.objects.get_docs(request.user, Document2, qfilter=Q(extra__startswith='query')) - if not d.content_object.is_history and d.content_object.type == 'query-' + editor_type - ] - else: - if USE_NEW_EDITOR.get(): - notebooks = [doc.to_dict() for doc in Document2.objects.documents(user=request.user).search_documents(types=['notebook'])] - else: - notebooks = [ - d.content_object.to_dict() - for d in Document.objects.get_docs(request.user, Document2, qfilter=Q(extra='notebook')) - if not d.content_object.is_history - ] - - return render('notebooks.mako', request, { - 'notebooks_json': json.dumps(notebooks, cls=JSONEncoderForHTML), - 'editor_type': editor_type - }) - - @check_document_access_permission def notebook(request, is_embeddable=False): if not SHOW_NOTEBOOKS.get() or not request.user.has_hue_permission(action="access", app='notebook'): @@ -99,8 +64,6 @@ def notebook(request, is_embeddable=False): LOG.exception('Spark is not enabled') return render('notebook.mako', request, { - 'editor_id': notebook_id or None, - 'notebooks_json': '{}', 'is_embeddable': request.GET.get('is_embeddable', False), 'options_json': json.dumps({ 'languages': get_ordered_interpreters(request.user), @@ -148,8 +111,6 @@ def editor(request, is_mobile=False, is_embeddable=False): template = 'editor_m.mako' return render(template, request, { - 'editor_id': editor_id or None, - 'notebooks_json': '{}', 'is_embeddable': request.GET.get('is_embeddable', False), 'editor_type': editor_type, 'options_json': json.dumps({ @@ -208,7 +169,6 @@ def browse(request, database, table, partition_spec=None): compute=compute ) return render('editor2.mako' if ENABLE_HUE_5.get() else 'editor.mako', request, { - 'notebooks_json': json.dumps([editor.get_data()]), 'options_json': json.dumps({ 'languages': get_ordered_interpreters(request.user), 'mode': 'editor', @@ -295,7 +255,6 @@ def execute_and_watch(request): raise PopupException(_('Action %s is unknown') % action) return render('editor2.mako' if ENABLE_HUE_5.get() else 'editor.mako', request, { - 'notebooks_json': json.dumps([editor.get_data()]), 'options_json': json.dumps({ 'languages': [{"name": "%s SQL" % editor_type.title(), "type": editor_type}], 'mode': 'editor',