[GitHub] [brooklyn-ui] jathanasiou commented on a change in pull request #263: Parameter defaults

[GitBox <gi...@apache.org>]

jathanasiou commented on a change in pull request #263:
URL: https://github.com/apache/brooklyn-ui/pull/263#discussion_r683551336



##########
File path: ui-modules/blueprint-composer/app/components/spec-editor/spec-editor.template.html
##########
@@ -323,7 +323,7 @@ <h4>No matching parameters</h4>
           <div ng-if="model.miscData.get('config').length === 0">
             <h4>No configuration</h4>
             <p class="buttons">
-                <button class="btn btn-sm btn-success" ng-click="specEditor.addConfigKey(state.config.search)" ng-if="state.config.search">
+                <button class="btn btn-sm btn-success" ng-click="specEditor.addConfigKey(state.config.search)" ng-if="state.config.search && state.config.search!=''">

Review comment:
       Don't believe this is needed as it adds complexity and  empty `''` strings evaluate to false in `if` statements anyways. https://stackoverflow.com/questions/27380000/angularjs-ng-if-check-string-empty-value

##########
File path: ui-modules/blueprint-composer/app/components/util/model/entity.model.js
##########
@@ -685,7 +685,17 @@ function addConfigKeyDefinition(param, overwrite, skipUpdatesDuringBatch) {
         let key = (param || {}).name;
         if (!key) throw new Error("'name' field must be included when adding parameter; was", param);
 
-        allConfig[key] = Object.assign(allConfig[key] || {}, param, overwrite ? null : allConfig[key]);
+        let paramMapped = Object.assign({}, param);

Review comment:
       I feel confident the below would be a much cleaner version of this function with the same behaviour. I'd really suggest something close for maintenance purposes if it feels accurate logic-wise.
   
   ```javascript
   function addConfigKeyDefinition(param, overwrite, skipUpdatesDuringBatch) {
       const allConfig = this.miscDataOrDefault('configMap', {});
       
       if (typeof param !== 'undefined') {
           
           const parameterItem = (typeof param !== 'string')
               ? { ...param } // shallow copy as object
               : {
                   "name": param,
                   "label": param,
                   "description": "",
                   "priority": 1,
                   "pinned": true,
                   "type": "java.lang.String",
                   "constraints": [],
               };
   
           const key = get(parameterItem, 'name'); // import { get } from 'lodash' at top, defaults to undef.
   
           // this will also throw if original `param` is neither object nor string due to previous statement
           if (!key) throw new Error("'name' field must be included when adding parameter; was", param);
   
           if (typeof parameterItem.default !== 'undefined') {
               /* Annoyingly, in parameters, we call the default value "default",
                * but in config, we call them "defaultValue";
                * when params are merged to config we need to rename
                */
               parameterItem.defaultValue = parameterItem.default;
               delete parameterItem.default;
           }
   
           Object.assign(allConfig[key], parameterItem);
       }
   
       if (!skipUpdatesDuringBatch) {
           this.miscData.set('config', Object.values(allConfig));
       }
   
       this.touch();
       return this;
   }
   ```

##########
File path: ui-modules/blueprint-composer/app/components/spec-editor/spec-editor.template.html
##########
@@ -332,7 +332,7 @@ <h4>No configuration</h4>
             <h4>No matching configuration</h4>
             <p class="buttons">
                 <button class="btn btn-sm btn-default" ng-if="state.config.search.length > 0" ng-click="state.config.search = ''">Clear search</button>
-                <button class="btn btn-sm btn-default" ng-click="specEditor.addConfigKey(state.config.search)" ng-if="!specEditor.getConfig(state.config.search)">
+                <button class="btn btn-sm btn-default" ng-click="specEditor.addConfigKey(state.config.search)" ng-if="state.config.search && state.config.search!='' && !specEditor.getConfig(state.config.search)">

Review comment:
       Same as previous.




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

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

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