Repository: zeppelin Updated Branches: refs/heads/master 341883d9e -> c016062ed
[ZEPPELIN-2670] fix: DON'T reset all helium config when saving display order ### What is this PR for? Reordering vis package using the `save` button resets all helium configuration. Originally, reported in [ZEPPELIN-2656](https://issues.apache.org/jira/browse/ZEPPELIN-2656) but created a new issue track that problem only. ### What type of PR is it? [Bug Fix] ### Todos DONE ### What is the Jira issue? [ZEPPELIN-2670](https://issues.apache.org/jira/browse/ZEPPELIN-2670) ### How should this be tested? 1. Enable 2 vis packages 2. Change display order 3. Refresh (**necessary**) the page 4. Change display order again ### Screenshots (if appropriate) #### before  #### after  ### Questions: * Does the licenses files need update? - NO * Is there breaking changes for older versions? - NO * Does this needs documentation? - NO Author: 1ambda <1am...@gmail.com> Closes #2424 from 1ambda/ZEPPELIN-2670/saving-helium-vis-order-should-not-reset and squashes the following commits: 04e046e8 [1ambda] fix: DON'T reset when getting display order 86d960c1 [1ambda] fix: DON'T call unncessary init() 7159a965 [1ambda] fix: Fetch bundleOrder after getAllPackages 13bb6508 [1ambda] fix: DON'T build when set orders d1aea4e8 [1ambda] style: reindent saveBundleOrder bb66c2bc [1ambda] style: Rename setVisualizationPackageOrder 8edfdf75 [1ambda] style: move getVisualizationPackageOrder func Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/c016062e Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/c016062e Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/c016062e Branch: refs/heads/master Commit: c016062edf42882860d4d51ee5b4b53aa7443621 Parents: 341883d Author: 1ambda <1am...@gmail.com> Authored: Wed Jun 21 19:45:06 2017 +0900 Committer: 1ambda <1am...@gmail.com> Committed: Tue Jun 27 18:17:29 2017 +0900 ---------------------------------------------------------------------- .../org/apache/zeppelin/rest/HeliumRestApi.java | 16 ++++---- .../src/app/helium/helium.controller.js | 34 +++++++++-------- .../src/components/helium/helium.service.js | 1 + .../java/org/apache/zeppelin/helium/Helium.java | 39 +++++++++++--------- .../org/apache/zeppelin/helium/HeliumConf.java | 5 ++- 5 files changed, 51 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c016062e/zeppelin-server/src/main/java/org/apache/zeppelin/rest/HeliumRestApi.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/HeliumRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/HeliumRestApi.java index 44b583c..1342f42 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/HeliumRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/HeliumRestApi.java @@ -211,13 +211,6 @@ public class HeliumRestApi { } @GET - @Path("order/visualization") - public Response getVisualizationPackageOrder() { - List<String> order = helium.setVisualizationPackageOrder(); - return new JsonResponse(Response.Status.OK, order).build(); - } - - @GET @Path("spell/config/{packageName}") public Response getSpellConfigUsingMagic(@PathParam("packageName") String packageName) { if (StringUtils.isEmpty(packageName)) { @@ -309,9 +302,16 @@ public class HeliumRestApi { return new JsonResponse(Response.Status.OK, packageConfig).build(); } + @GET + @Path("order/visualization") + public Response getVisualizationPackageOrder() { + List<String> order = helium.getVisualizationPackageOrder(); + return new JsonResponse(Response.Status.OK, order).build(); + } + @POST @Path("order/visualization") - public Response getVisualizationPackageOrder(String orderedPackageNameList) { + public Response setVisualizationPackageOrder(String orderedPackageNameList) { List<String> orderedList = gson.fromJson( orderedPackageNameList, new TypeToken<List<String>>(){}.getType()); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c016062e/zeppelin-web/src/app/helium/helium.controller.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/helium/helium.controller.js b/zeppelin-web/src/app/helium/helium.controller.js index 5fcf2c6..a610fb6 100644 --- a/zeppelin-web/src/app/helium/helium.controller.js +++ b/zeppelin-web/src/app/helium/helium.controller.js @@ -50,16 +50,18 @@ export default function HeliumCtrl ($scope, $rootScope, $sce, }) .then(defaultPackageConfigs => { $scope.defaultPackageConfigs = defaultPackageConfigs + return heliumService.getVisualizationPackageOrder() }) - - // 2. get vis package order - heliumService.getVisualizationPackageOrder() .then(visPackageOrder => { - $scope.bundleOrder = visPackageOrder - $scope.bundleOrderChanged = false + setVisPackageOrder(visPackageOrder) }) } + const setVisPackageOrder = function(visPackageOrder) { + $scope.bundleOrder = visPackageOrder + $scope.bundleOrderChanged = false + } + let orderPackageByPubDate = function (a, b) { if (!a.pkg.published) { // Because local registry pkgs don't have 'published' field, put current time instead to show them first @@ -133,18 +135,18 @@ export default function HeliumCtrl ($scope, $rootScope, $sce, confirm.$modalFooter.find('button:contains("OK")') .html('<i class="fa fa-circle-o-notch fa-spin"></i> Enabling') heliumService.setVisualizationPackageOrder($scope.bundleOrder) - .success(function (data, status) { - init() - confirm.close() - }) - .error(function (data, status) { - confirm.close() - console.log('Failed to save order') - BootstrapDialog.show({ - title: 'Error on saving order ', - message: data.message + .success(function (data, status) { + setVisPackageOrder($scope.bundleOrder) + confirm.close() + }) + .error(function (data, status) { + confirm.close() + console.log('Failed to save order') + BootstrapDialog.show({ + title: 'Error on saving order ', + message: data.message + }) }) - }) return false } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c016062e/zeppelin-web/src/components/helium/helium.service.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/components/helium/helium.service.js b/zeppelin-web/src/components/helium/helium.service.js index c26f95b..d1c1077 100644 --- a/zeppelin-web/src/components/helium/helium.service.js +++ b/zeppelin-web/src/components/helium/helium.service.js @@ -126,6 +126,7 @@ export default function heliumService ($http, $sce, baseUrlSrv) { this.getAllPackageInfo = function () { return $http.get(`${baseUrlSrv.getRestApiBase()}/helium/package`) .then(function (response, status) { + console.warn(Object.keys(response.data.body).length) return response.data.body }) .catch(function (error) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c016062e/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java index f5f6695..08014c4 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java @@ -309,13 +309,21 @@ public class Helium { return; } - // if package is visualization, rebuild bundle + // if package is bundle, rebuild bundle if (HeliumPackage.isBundleType(pkgInfo.getPkg().getType())) { bundleFactory.buildPackage(pkgInfo.getPkg(), true, true); } - // update conf and save + // set `enable` field heliumConf.enablePackage(name, artifact); + // set display order + if (pkgInfo.getPkg().getType() == HeliumType.VISUALIZATION) { + List<String> currentDisplayOrder = heliumConf.getBundleDisplayOrder(); + if (!currentDisplayOrder.contains(name)) { + currentDisplayOrder.add(name); + } + } + save(); } @@ -326,8 +334,16 @@ public class Helium { return; } - // update conf and save + HeliumPackageSearchResult pkgInfo = getPackageInfo(name, artifact); + + // set `enable` field heliumConf.disablePackage(name); + if (pkgInfo.getPkg().getType() == HeliumType.VISUALIZATION) { + List<String> currentDisplayOrder = heliumConf.getBundleDisplayOrder(); + if (currentDisplayOrder.contains(name)) { + currentDisplayOrder.remove(name); + } + } save(); } @@ -437,26 +453,13 @@ public class Helium { * Get enabled package list in order * @return */ - public List<String> setVisualizationPackageOrder() { - List orderedPackageList = new LinkedList<>(); - List<HeliumPackage> packages = getBundlePackagesToBundle(); - - for (HeliumPackage pkg : packages) { - if (HeliumType.VISUALIZATION == pkg.getType()) { - orderedPackageList.add(pkg.getName()); - } - } - - return orderedPackageList; + public List<String> getVisualizationPackageOrder() { + return heliumConf.getBundleDisplayOrder(); } public void setVisualizationPackageOrder(List<String> orderedPackageList) throws IOException { heliumConf.setBundleDisplayOrder(orderedPackageList); - - // if package is visualization, rebuild buildBundle - bundleFactory.buildAllPackages(getBundlePackagesToBundle()); - save(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c016062e/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumConf.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumConf.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumConf.java index fc341d5..9d32212 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumConf.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/HeliumConf.java @@ -31,7 +31,8 @@ public class HeliumConf { new HashMap<String, Map<String, Object>>()); // enabled visualization package display order - private List<String> bundleDisplayOrder = new LinkedList<>(); + private List<String> bundleDisplayOrder = + Collections.synchronizedList(new LinkedList<String>()); public Map<String, String> getEnabledPackages() { return new HashMap<>(enabled); @@ -88,6 +89,6 @@ public class HeliumConf { } public void setBundleDisplayOrder(List<String> orderedPackageList) { - bundleDisplayOrder = orderedPackageList; + bundleDisplayOrder = Collections.synchronizedList(orderedPackageList); } }
