You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/05/28 17:37:24 UTC

[GitHub] [brooklyn-ui] algairim opened a new pull request #206: Allow downstream to configure edit.dsl controller

algairim opened a new pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206


   Signed-off-by: Mykola Mandra <my...@cloudsoftcorp.com>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r642444496



##########
File path: ui-modules/blueprint-composer/app/views/main/graphical/edit/dsl/edit.dsl.controller.js
##########
@@ -131,27 +127,31 @@ export const graphicalEditDslState = {
         index: {
             value: '',
             squash: true
-        }
+        },
+        isConfig: true // This flag identifies whether DSL edit is for configuration or something else. Configuration is a default.

Review comment:
       Changed to subsection.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] asfgit merged pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r642386043



##########
File path: ui-modules/blueprint-composer/app/views/main/graphical/edit/dsl/edit.dsl.controller.js
##########
@@ -131,27 +127,31 @@ export const graphicalEditDslState = {
         index: {
             value: '',
             squash: true
-        }
+        },
+        isConfig: true // This flag identifies whether DSL edit is for configuration or something else. Configuration is a default.

Review comment:
       This flag is not used in Brooklyn code just now, however, branding parts rely on it. For example, adding DSL editor menu for a non-config case:
   ```
   <a ui-sref="main.graphical.edit.dsl({entityId: model._id, for: item.name, isConfig: false})" class="btn btn-default" title="Open in DSL editor"><i class="fa fa-bolt"></i></a>
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r642535993



##########
File path: ui-modules/blueprint-composer/app/components/dsl-editor/dsl-editor.js
##########
@@ -368,7 +368,7 @@ export function dslEditorDirective($rootScope, $filter, $log, brUtilsGeneral, bl
             }
         }
 
-        return new Dsl(KIND.METHOD, 'component').param(new Dsl(KIND.STRING, targetEntity.id));
+        return new Dsl(KIND.TARGET, TARGET.COMPONENT).param(new Dsl(KIND.STRING, targetEntity.id));

Review comment:
       DSL of kind METHOD referring to COMPONENT target created on select by default, however, it did not make sense, I changed this to be kind TARGET. @tbouron, could you confirm that this is correct? I did not find a test to break with this change.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#issuecomment-850570230


   There are no functional changes in this PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r642386043



##########
File path: ui-modules/blueprint-composer/app/views/main/graphical/edit/dsl/edit.dsl.controller.js
##########
@@ -131,27 +127,31 @@ export const graphicalEditDslState = {
         index: {
             value: '',
             squash: true
-        }
+        },
+        isConfig: true // This flag identifies whether DSL edit is for configuration or something else. Configuration is a default.

Review comment:
       This flag is not used in Brooklyn code just now, however, branding parts rely on it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r641711647



##########
File path: ui-modules/blueprint-composer/app/views/main/graphical/edit/dsl/edit.dsl.controller.js
##########
@@ -131,27 +127,31 @@ export const graphicalEditDslState = {
         index: {
             value: '',
             squash: true
-        }
+        },
+        isConfig: true // This flag identifies whether DSL edit is for configuration or something else. Configuration is a default.
     },
     template: template,
     controller: ['$scope', '$state', '$stateParams', 'objectCache', 'state', EditDslController],
     controllerAs: 'vm',
     resolve: {
-        state: ['$state', '$stateParams', 'entity', 'brSnackbar', 'objectCache', ($state, $stateParams, entity, brSnackbar, objectCache) => {
-            let definition = entity.miscData.get('config').find(config => config.name === $stateParams.for);
+        state: ['$state', '$stateParams', 'entity', 'brSnackbar', 'objectCache', 'composerOverrides', ($state, $stateParams, entity, brSnackbar, objectCache, composerOverrides) => {
+
+            // Initialize dsl-edit helpers
+            $state.getDefinition = getConfigDefinition;
+            $state.getRootDsl = getConfigRootDsl;
+            $state.setRootDsl = setConfigRootDsl;
+
+            // Allow downstream to configure this controller and override helpers defined above, when required.
+            (composerOverrides.configureDslEdit || function () {})($state);

Review comment:
       This is a new feature to enable configuration of this controller, e.g. branding.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] ahgittin commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r642392047



##########
File path: ui-modules/blueprint-composer/app/views/main/graphical/edit/dsl/edit.dsl.controller.js
##########
@@ -131,27 +127,31 @@ export const graphicalEditDslState = {
         index: {
             value: '',
             squash: true
-        }
+        },
+        isConfig: true // This flag identifies whether DSL edit is for configuration or something else. Configuration is a default.

Review comment:
       would be nicer to call this something like `subsection` which might take values `config`, `location`, etc




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r642535993



##########
File path: ui-modules/blueprint-composer/app/components/dsl-editor/dsl-editor.js
##########
@@ -368,7 +368,7 @@ export function dslEditorDirective($rootScope, $filter, $log, brUtilsGeneral, bl
             }
         }
 
-        return new Dsl(KIND.METHOD, 'component').param(new Dsl(KIND.STRING, targetEntity.id));
+        return new Dsl(KIND.TARGET, TARGET.COMPONENT).param(new Dsl(KIND.STRING, targetEntity.id));

Review comment:
       DSL of kind METHOD referring to COMPONENT target created on select by default, however, it did not make sense, I changed this to be kind TARGET. @tbouron, could confirm that this is correct? I did not find a test to break with this change.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] algairim commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r641711647



##########
File path: ui-modules/blueprint-composer/app/views/main/graphical/edit/dsl/edit.dsl.controller.js
##########
@@ -131,27 +127,31 @@ export const graphicalEditDslState = {
         index: {
             value: '',
             squash: true
-        }
+        },
+        isConfig: true // This flag identifies whether DSL edit is for configuration or something else. Configuration is a default.
     },
     template: template,
     controller: ['$scope', '$state', '$stateParams', 'objectCache', 'state', EditDslController],
     controllerAs: 'vm',
     resolve: {
-        state: ['$state', '$stateParams', 'entity', 'brSnackbar', 'objectCache', ($state, $stateParams, entity, brSnackbar, objectCache) => {
-            let definition = entity.miscData.get('config').find(config => config.name === $stateParams.for);
+        state: ['$state', '$stateParams', 'entity', 'brSnackbar', 'objectCache', 'composerOverrides', ($state, $stateParams, entity, brSnackbar, objectCache, composerOverrides) => {
+
+            // Initialize dsl-edit helpers
+            $state.getDefinition = getConfigDefinition;
+            $state.getRootDsl = getConfigRootDsl;
+            $state.setRootDsl = setConfigRootDsl;
+
+            // Allow downstream to configure this controller and override helpers defined above, when required.
+            (composerOverrides.configureDslEdit || function () {})($state);

Review comment:
       This is a new feature to enable configuration of this controller, e.g. branding. No functional change.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] ahgittin commented on pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#issuecomment-851554152


   LGTM, merging.  thx for the confirmation @tbouron re TARGET v METHOD


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-ui] tbouron commented on a change in pull request #206: Allow downstream to configure edit.dsl controller

Posted by GitBox <gi...@apache.org>.
tbouron commented on a change in pull request #206:
URL: https://github.com/apache/brooklyn-ui/pull/206#discussion_r642552025



##########
File path: ui-modules/blueprint-composer/app/components/dsl-editor/dsl-editor.js
##########
@@ -368,7 +368,7 @@ export function dslEditorDirective($rootScope, $filter, $log, brUtilsGeneral, bl
             }
         }
 
-        return new Dsl(KIND.METHOD, 'component').param(new Dsl(KIND.STRING, targetEntity.id));
+        return new Dsl(KIND.TARGET, TARGET.COMPONENT).param(new Dsl(KIND.STRING, targetEntity.id));

Review comment:
       It's been a while since I touched that code. It looks functionally the same so that should work @algairim, but I cannot give you a 100% guarantee 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org