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/06/21 14:58:58 UTC

[GitHub] [brooklyn-ui] iuliana opened a new pull request #229: Added fix for issue smart-204, version not being updated in the Add ...

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


   to catalog modal. (More like, refactored a little to the fix can be aplied on implementation provided by `composerOverrides`)


-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the branding part and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -93,22 +117,10 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
             }
             if (!$scope.isNewFromTemplate()) {
                 // (these should only be set if not making something new from a template, as the entity items will refer to the template)
-
-                // the name and the ID can be set in the UI, 
-                // or all can be inherited if root node is a known application type we are editting 
-                // (normally in those cases $scope.config will already be set by caller, but maybe not always) 
-                if (!$scope.config.name && entity.hasName()) {
-                    $scope.config.name = entity.name;
-                }
-                if (!$scope.config.symbolicName && (entity.hasId() || metadata.has('id'))) {
-                    $scope.config.symbolicName = entity.id || metadata.get('id');
-                }
+                (composerOverrides.getConfigNameFromEntity || getConfigNameFromEntity)(entity, $scope);
+                (composerOverrides.getSuggestedSymbolicNameFromBlueprint || getSuggestedSymbolicNameFromBlueprint)(entity, metadata, $scope);
                 (composerOverrides.getSuggestedVersionToSaveFromBlueprint || getSuggestedVersionToSaveFromBlueprint)(entity, metadata, $scope);
-                if (!$scope.config.bundle) {
-                    if ($scope.config.symbolicName) {
-                        $scope.config.bundle = $scope.config.symbolicName;
-                    }
-                }
+                (composerOverrides.getConfigBundleNameFromScope || getConfigBundleNameFromScope)($scope);

Review comment:
       Grouping them together looks better.




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the downstream code and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the downstream code and call them in this code accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the downstream code and call them in this code by accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the branding part and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   




-- 
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] iuliana commented on pull request #229: Added fix for issue smart-204, version not being updated in the Add ...

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


   The test failing is one of our flaky ones.


-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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


   methods starting `get...` i expect to return the value, not write the value in scope.  `populateXxxx` or `setXxxx` might be better choices.
   
   otherwise looks good.
   
   i also prefer @algairim 's pattern but don't feel strongly.


-- 
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] iuliana commented on a change in pull request #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       Tried it, the pattern cannot be applied to this implementation.




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the branding overlay and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       Correction: it is better to define methods above at the controller level, not at directive:
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       return {
           restrict: 'E',
           templateUrl: function (tElement, tAttrs) {
               return tAttrs.templateUrl || TEMPLATE_URL;
           },
           scope: {
               config: '=',
           },
           controller: ['$scope','composerOverrides', saveToCatalogModalController],
           link: link
       };
   
       function saveToCatalogModalController($scope, composerOverrides) {
           let modalController = this;
   
           modalController.getConfigNameFromEntity = (entity) => {
   
               // use $scope directly
               // code omitted ...
   
           };
           
           // etc. ...
   
           // allow downstream to configure directive and scope here
           (composerOverrides.configureCatalogModal || function () {})(modalController, $scope);
       }
   ```




-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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


   methods starting `get...` i expect to return the value, not write the value in scope.  `populateXxxx` or `setXxxx` might be better choices.
   
   otherwise looks good.
   
   i also prefer @algairim 's pattern but don't feel strongly.


-- 
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] iuliana merged pull request #229: Added fix for issue smart-204, version not being updated in the Add ...

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


   


-- 
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 #229: Added fix for issue smart-204, version not being updated in the Add ...

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



##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the branding part and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   

##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the branding part and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   

##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the branding overlay and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   

##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the downstream code and call them accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   

##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the downstream code and call them in this code accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   

##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       I would not include so many custom calls from the override provider.
    
   Suggestion:
   
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       let modalDirective = this;
   
       modalDirective.getConfigNameFromEntity = (entity) => {
       
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getConfigBundleNameFromScope = () => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.getSuggestedSymbolicNameFromBlueprint = (entity, metadata) => {
   
       // use $scope directly
       // code omitted ...
   
       };
   
       modalDirective.updateCatalogConfig = (config, $element) => {
   
       // use $scope directly
       // code omitted ...
   
       }
   
       // allow downstream to configure directive and scope here
       (composerOverrides.configureCatalog || () => {})(modalDirective, $scope);
   
       return = {
   
       // code omitted ...
   ```
   
   Customize methods defined on `modalDirective` instance in the downstream code and call them in this code by accessing instance of the directive `modalDirective.getConfigNameFromEntity(entity)`
   

##########
File path: ui-modules/blueprint-composer/app/components/catalog-saver/catalog-saver.directive.js
##########
@@ -68,6 +68,30 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $f
         }
     }
 

Review comment:
       Correction: it is better to define methods above at the controller level, not at directive:
   ```
   export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, $filter, composerOverrides, blueprintService) {
       return {
           restrict: 'E',
           templateUrl: function (tElement, tAttrs) {
               return tAttrs.templateUrl || TEMPLATE_URL;
           },
           scope: {
               config: '=',
           },
           controller: ['$scope','composerOverrides', saveToCatalogModalController],
           link: link
       };
   
       function saveToCatalogModalController($scope, composerOverrides) {
           let modalController = this;
   
           modalController.getConfigNameFromEntity = (entity) => {
   
               // use $scope directly
               // code omitted ...
   
           };
           
           // etc. ...
   
           // allow downstream to configure directive and scope here
           (composerOverrides.configureCatalogModal || function () {})(modalController, $scope);
       }
   ```




-- 
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