Skip to content

Conversation

@rbordeanu
Copy link
Contributor

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

This change enables to externaly override node properties (i.e. on the incoming msg).
It also allows for mustache templating on the node property (i.e. use {{attribute}} in the node UI to explicitly reference a msg attribute).

I also cleaned up the code generation template in case of 'body' parameter, to reduce code clutter and make the generated code a bit more intuitive for human reading.

Checklist

Put an x in the boxes that apply

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the mailing list/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@coveralls
Copy link

coveralls commented Feb 28, 2018

Coverage Status

Coverage increased (+1.1%) to 74.932% when pulling b3a3802 on rbordeanu:pr/enable-override-node-parameters into 69ba8f9 on node-red:master.

@rbordeanu
Copy link
Contributor Author

[slight off-topic]: Any idea why my identation on the commit seems messed up?
I'm using VS Code for editing and it looks fine on my side.

@t-ide
Copy link
Contributor

t-ide commented Mar 1, 2018

Probably you have to use spaces instead of tab as indentation. 
You can use "editor.detectIndentation" option in VSCode settings. https://code.visualstudio.com/docs/getstarted/settings

@knolleary
Copy link
Member

We don’t do mustache templating in general - and I am not in favour of adding it here.
The http request url field is a rare exception - we added it very early on in node-red and can't now remove it. It is useful in that instance because you do often want to template the url - ie substitute just part of it with a msg property.

But if you just want to set a node property to the value of a msg property, then we’d normally now use a TypedInput that lets you pick a msg. property, or select string type and enter the desired value.

@rbordeanu
Copy link
Contributor Author

I updated the PR according to the recommendation from @knolleary (i.e changed param fields to typedInput) + added more helpers for code generation

@rbordeanu rbordeanu changed the title enable override of node properties and allow mustache templating enable override of node properties and clean up codegen template Mar 2, 2018
@kazuhitoyokoi
Copy link
Member

@t-ide Thank you for your help.
@knolleary Thank you for your recommendation about typedInput. I learned the design of Node-RED.
@rbordeanu I checked your code. And I generated and tested swagger node using petstore swagger definition. It looks great. 💯

typedinput

@HiroyasuNishiyama Could you merge this pull request?

@rbordeanu
Copy link
Contributor Author

rbordeanu commented Mar 3, 2018

I had to amend the PR because the naive way of retrieving msg property was not working for deep/nested properties (ex. msg.a.b.c).
Fixed using the correct method now, which is RED.util.getMessageProperty.

@HiroyasuNishiyama HiroyasuNishiyama merged commit 3537fa3 into node-red:master Mar 5, 2018
@kazuhitoyokoi
Copy link
Member

@HiroyasuNishiyama Thank you. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants