Add msg property payloadHandling for setting how a GET request handles payload

[Steve-Mcl]

Types of changes

As reported on forum and raised in #3957, the HTTP Request node could not set the payload handling for a get request.
The option set in the dropdown field before the method was changed to "use msg.method" and hidden would be used causing confusion to user.

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

Proposed changes

As described in the issue, this PR adds a msg property to permit the user to specify the payload handling via msg.payloadHandling`

Other changes include...

• Update help to include instructions on msg.payloadHandling property
• leave the "Payload Handling" dropdown visible for "- set by msg.method -" and "GET" options
• Add an entry to the "Payload Handling" dropdown named "- set by msg.payloadHandling -" to provide a visible cue
  

Considerations & points of discussion before merging...

1. Although setting body in a GET request is VERY uncommon (many web servers even prohibit this) there may be some flows in the wild that work "by accident" (having set the payload handling dropdown to "add payload to body" or "add payload to querystring") BEFORE setting the method dropdown to "Use msg.method". We may want to use the nodes value OR consider showing the payload handling dropdown & adding a "use msg.payloadHandling" entry?
   1. Updated PR in 3a5cdbc
2. As noted in comments in the tests: Either Express does not deliver the body of a GET request OR GOT doesn't send it! That means we cannot test this! Oddly, if the HTTP-Req node sets options.json instead of options.body when making the request, then the body is delivered! I was curious if this ever worked and fired up Node-RED v1.3.7 (using REQUEST instead of GOT) & it seems it may be express discarding the body. I have no way to prove this works other than to say, if the allowGetBody option is NOT set and we include a body, we get the error The 'GET' method cannot be used with a body! So we can only assume it works.

Checklist

• I have read the contribution guidelines
• For non-bugfix PRs, I have discussed this change on the forum/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]

Coverage increased (+0.02%) to 68.34% when pulling 20a63a3 on 3957-http-get-request-payload-option into 14c362d on master.

[knolleary]

I'm not 100% on the UX of this approach.

What if we just added support for always appending msg.urlQueryString to the url regardless of (or in addition to) whatever else has been configured)

Then for a user that wanted to use the request node with the method being set dynamically, they move their payload to that property to get it appended.

[Steve-Mcl]

Hi Nick. In the first commit there were no UI changes at all but I was unhappy with the "visibility" and "discovery" of the feature.

I will try to explain:

1. User would select Method: " - set by msg.method - "
   1. The "Payload:" option was hidden - leaving user unaware there was an option to include payload as an "append to querystring" or to set the requests body or even to completely ignore it.

What if we just added support for always appending msg.urlQueryString to the url regardless of (or in addition to) whatever else has been configured)

Then for a user that wanted to use the request node with the method being set dynamically, they move their payload to that property to get it appended.

It is a reasonable proposal unfortunately if we change this to a separate property (e.g. msg.requestData or msg.appendQuery) we would break any flow out there that currently works (because they set the dropdown first) since they would be expecting payload to be appended to the QS or set as the req body.

So that's also part of the reason I went on to add the UI parts making the bug fix backwards compatible and illuminate the ability to override while still respecting the users flow settings.

I'm not saying it couldn't be presented better - I am open to suggestions as always.

---

If we are dead set against UI changes, then perhaps a compromise?...

• Add support for msg property payloadHandling of "ignore", "query" & "body" (same as current dropdown)
• If payloadHandling is supplied in the msg, simply override the nodes config & apply the payload as prescribed.
  • NOTE: this is against recent trends of having to have the UI left blank to override (2 sections in the HTML node already emit warnings about not overriding UI props)