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

feat(HTTP Request Node): Port optimizeResponse from httpRequest tool to standalone node (no-changelog) #14307

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

CharlieKolb
Copy link
Contributor

@CharlieKolb CharlieKolb commented Apr 1, 2025

Summary

Introduce new tool-only property optimizeResponse which is equivalent to the toolHttpRequest option. Chose to duplicate the code rather than introducing all the dependencies into the workflow package.

Note that we'll enable this node as a tool (and hide the old version) in a separate PR after this.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/ADO-3304/feature-port-optimizeresponse-to-all-nodesastools

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
snyk-bot Snyk bot
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team node/improvement New feature or request labels Apr 1, 2025
@CharlieKolb CharlieKolb changed the title feat(HttpRequest Node): Port optimizeResponse from httpRequest tool to standalone node feat(HTTP Request Node): Port optimizeResponse from httpRequest tool to standalone node Apr 4, 2025
@CharlieKolb CharlieKolb changed the title feat(HTTP Request Node): Port optimizeResponse from httpRequest tool to standalone node feat(HTTP Request Node): Port optimizeResponse from httpRequest tool to standalone node (no-changelog) Apr 4, 2025
@CharlieKolb CharlieKolb marked this pull request as ready for review April 4, 2025 08:31
Copy link

codecov bot commented Apr 4, 2025

Codecov Report

Attention: Patch coverage is 16.66667% with 90 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...-base/nodes/HttpRequest/shared/optimizeResponse.ts 13.13% 86 Missing ⚠️
...es-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts 42.85% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@CharlieKolb CharlieKolb requested a review from RicardoE105 April 7, 2025 10:09
@@ -870,6 +870,7 @@
"@n8n/di": "workspace:*",
"@n8n/imap": "workspace:*",
"@n8n/vm2": "3.9.25",
"@mozilla/readability": "0.5.0",
Copy link
Member

Choose a reason for hiding this comment

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

we are already using 0.6.0 in the langchain packahe

};
};

export const configureResponseOptimizer = (ctx: IExecuteFunctions, itemIndex: number) => {
Copy link
Member

Choose a reason for hiding this comment

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

should we add some unit tests for this?

@@ -1167,6 +1169,10 @@ export const mainProperties: INodeProperties[] = [
},
],
},
...optimizeResponseProperties.map((x) => ({
Copy link
Member

Choose a reason for hiding this comment

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

instead of adding these here, we could define a new flag in node descriptions, and when that flag is set, we could add these properties in applySpecialNodeParameters.
That way we can add this as an option on more nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team node/improvement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants