Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UI for Http Request node headers #3488

Merged
merged 6 commits into from
Apr 6, 2022

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented Mar 12, 2022

fixes #3428

Types of changes

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

Proposed changes

  • Add UI for headers
    chrome_lDT3SGgrvS
  • Update built in help
  • Add test - "should use ui headers"
    • verifies the RULES stated below
  • Modify test - "should upload a file" to use UI entry for Content-Type instead of passing it in via msg.headers

RULES: msg.headers vs UI headers...

  • If header "FOO": "BAR" is sent in msg.headers and is NOT present in the UI, then "FOO" is added to req.headers
  • If header "FOO": "BAR" is sent in msg.headers and is "FOO": "BAZ" in the UI, then the "FOO" header is overwritten with "BAZ" in req.headers
  • If header "FOO": "QUX" is sent in msg.headers and is "FOO": "" (i.e. no value) in the UI, then "FOO" is deleted from req.headers

NOTES...

  • Header names are set in the case specified by the user
  • UI Header names are Camel Case by default
    • If user wants to send content-type (lower-case) instead of Content-Type (Camel Case) then can either add it to msg.headers or add a UI entry set to "other" and name it content-type

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

- Wrap HTML node script in IFFE (isolate module level vars & functions)
- Add UI elements for setting headers in http req node edit form
- Update built in help
- Add tests
- remove Accept-Charset
- Use camel case by default
- additional encodings
- use `this` not `node` in UI change events
@Steve-Mcl Steve-Mcl linked an issue Mar 12, 2022 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Mar 12, 2022

Coverage Status

Coverage increased (+1.5%) to 68.778% when pulling 5fda20c on Steve-Mcl:httpreq-headers-ui into 6a5c50f on node-red:dev.

- Revert last minute code tidy that changed too many `this` to `node`
@knolleary knolleary added this to the 3.0 milestone Mar 14, 2022
Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor help text tweak needed, but otherwise looks good.

Happy for you to merge once the help is updated.

Co-authored-by: Nick O'Leary <nick.oleary@gmail.com>
@Steve-Mcl Steve-Mcl merged commit a7932da into node-red:dev Apr 6, 2022
@Steve-Mcl Steve-Mcl deleted the httpreq-headers-ui branch April 6, 2022 07:35
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.

feature: HTTP Request: Add options for common response headers
3 participants