You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2018/11/16 13:44:01 UTC
[GitHub] brooklyn-ui pull request #109: catalog saver to support 'application', 'temp...
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-ui/pull/109
catalog saver to support 'application', 'template', and 'entity'
in line with https://github.com/apache/brooklyn-server/pull/1015
also when using a template the metadata is not populated as the intention is probably not to overwrite,
unless the user selects to save it as a template
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-ui catalog-saver-trimodal
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-ui/pull/109.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #109
----
commit a552f04640459669cabdd5882b9e8964998b746e
Author: Alex Heneveld <al...@...>
Date: 2018-11-16T13:42:17Z
catalog saver to support 'application', 'template', and 'entity'
in line with https://github.com/apache/brooklyn-server/pull/1015
also when using a template the metadata is not populated as the intention is probably not to overwrite,
unless the user selects to save it as a template
----
---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-ui/pull/109
@ahgittin for 'template', is that really what it means in the product from a user's perspective? Something declared as a 'template' will appear in the quick launch, whereas other things will not. When you click on it in the 'quick launch', then a dialog pops up with two buttons: `deploy` and `open in editor`. The latter opens a simple text editor in the quick-launch dialog, rather than re-directing to the blueprint composer.
Your description of 'template' is accurate for when this was originally added to the code base, but I don't think that is what it does now, or how it is used now. That description won't make sense to a user, based on how the Brooklyn UI behaves.
---
[GitHub] brooklyn-ui pull request #109: catalog saver to support 'application', 'temp...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-ui/pull/109#discussion_r234937789
--- Diff: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js ---
@@ -42,14 +42,15 @@ const TYPES = [
];
angular.module(MODULE_NAME, [angularAnimate, uibModal, brUtils])
- .directive('catalogSaver', ['$rootScope', '$uibModal', '$injector', 'composerOverrides', 'blueprintService', saveToCatalogModalDirective])
+ .directive('catalogSaver', ['$rootScope', '$uibModal', '$injector', '$filter', 'composerOverrides', 'blueprintService', saveToCatalogModalDirective])
.directive('catalogVersion', ['$parse', catalogVersionDirective])
+ .directive('blueprintNameOrSymbolicNameAndBundleIdRequired', blueprintNameOrSymbolicNameAndBundleIdRequiredDirective)
--- End diff --
okay, compromise, changed to `composerBlueprintNameValidator`
---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-ui/pull/109
@aledsage Are you happy to address your comment in a subsequent PR? I'm ok to merge this one but would like to check with you first.
---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-ui/pull/109
Sure, happy to merge and we can make subsequent improvements in other PRs. Merging now.
---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-ui/pull/109
one other change, we weren't watching `bundle` and `symbolicName` so the validator didn't do everything it should, that's fixed too now
---
[GitHub] brooklyn-ui pull request #109: catalog saver to support 'application', 'temp...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-ui/pull/109
---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-ui/pull/109
@tbouron the itemType is included in the tooltip text, in parentheses at the end
@aledsage i think you're flagging a problem with quick launch that it is cheating a bit using the `catalog_template` tag for what should be a special `quick_launch` or favourites mechanism. that should be fixed as a separate task. but until we've done this i'll add to that tooltip text something like that "In some contexts this prioritizes the blueprint for inclusion in quick-selection views."
---
[GitHub] brooklyn-ui pull request #109: catalog saver to support 'application', 'temp...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-ui/pull/109#discussion_r234939255
--- Diff: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js ---
@@ -42,14 +42,15 @@ const TYPES = [
];
angular.module(MODULE_NAME, [angularAnimate, uibModal, brUtils])
- .directive('catalogSaver', ['$rootScope', '$uibModal', '$injector', 'composerOverrides', 'blueprintService', saveToCatalogModalDirective])
+ .directive('catalogSaver', ['$rootScope', '$uibModal', '$injector', '$filter', 'composerOverrides', 'blueprintService', saveToCatalogModalDirective])
.directive('catalogVersion', ['$parse', catalogVersionDirective])
+ .directive('blueprintNameOrSymbolicNameAndBundleIdRequired', blueprintNameOrSymbolicNameAndBundleIdRequiredDirective)
--- End diff --
Sounds good 👍
---
[GitHub] brooklyn-ui pull request #109: catalog saver to support 'application', 'temp...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-ui/pull/109#discussion_r234937837
--- Diff: ui-modules/blueprint-composer/app/views/main/main.controller.js ---
@@ -105,16 +105,31 @@ export function MainController($scope, $element, $log, $state, $stateParams, brB
vm.saveToCatalogConfig = {};
if (edit) {
+ console.log("edit", edit);
--- End diff --
good spot
---
[GitHub] brooklyn-ui pull request #109: catalog saver to support 'application', 'temp...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-ui/pull/109#discussion_r234928083
--- Diff: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js ---
@@ -42,14 +42,15 @@ const TYPES = [
];
angular.module(MODULE_NAME, [angularAnimate, uibModal, brUtils])
- .directive('catalogSaver', ['$rootScope', '$uibModal', '$injector', 'composerOverrides', 'blueprintService', saveToCatalogModalDirective])
+ .directive('catalogSaver', ['$rootScope', '$uibModal', '$injector', '$filter', 'composerOverrides', 'blueprintService', saveToCatalogModalDirective])
.directive('catalogVersion', ['$parse', catalogVersionDirective])
+ .directive('blueprintNameOrSymbolicNameAndBundleIdRequired', blueprintNameOrSymbolicNameAndBundleIdRequiredDirective)
--- End diff --
Wow, that a very loooooong name. Could we come up with something shorter like `blueprintNameValidator`? The name doesn't need to say what it does exactly
---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-ui/pull/109
@ahgittin I'm wondering if the tooltip texts should include the item type, the same way you describe them in your previous comment. I think it's a good compromise so users have a reference they can scan for.
---
[GitHub] brooklyn-ui issue #109: catalog saver to support 'application', 'template', ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-ui/pull/109
@tbouron agree it would be nice to make the differences clearer but not sure how. the test to apply is whether it's clearer than before where we prompted for "Application" or "Entity" with no difference, and I think it is. i've expanded the wording slightly, and welcome further enhancements, but anything more than quick fixes I think will be a longer discussion and shouldn't block this:
* *Application entity*: Save as an application entity which can be deployed on its own, or configured and used in blueprints but only config and sensors declared at the root are accessible in the Composer ('application' item type)
* *Application template*: Save as a blueprint template which can be used as an editable starting point for blueprints or used as an application entity ('template' item type)
* *Extended entity*: Save as an entity which can be configured and used in blueprints, exposing the config and adjuncts it inherits ('entity' item type)
---
[GitHub] brooklyn-ui pull request #109: catalog saver to support 'application', 'temp...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-ui/pull/109#discussion_r234928669
--- Diff: ui-modules/blueprint-composer/app/views/main/main.controller.js ---
@@ -105,16 +105,31 @@ export function MainController($scope, $element, $log, $state, $stateParams, brB
vm.saveToCatalogConfig = {};
if (edit) {
+ console.log("edit", edit);
--- End diff --
To remove
---