Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(n8n Form Trigger Node): Improvements #10092

Merged
merged 30 commits into from
Jul 29, 2024

Conversation

michael-radency
Copy link
Contributor

@michael-radency michael-radency commented Jul 18, 2024

Summary

Basic Authentication
Placeholders
Query params as defaults in form
Ignore bots option
Format date
Email field type
File field type
Required fields marked with *
Description supports line breaks (\n, <br>)
Fixed submittedAt property to be in workflow timezone
Improved small screens view
Various styles fixes

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/NODE-1505/form-trigger-improvements-p1
https://community.n8n.io/t/n8n-form-trigger-upload-files-type-field/32495/5
https://community.n8n.io/t/timezone-ignored-in-webform-submittedat-time/46685/10
https://community.n8n.io/t/n8n-form-size/34877

@michael-radency michael-radency added node/improvement New feature or request n8n team Authored by the n8n team node/issue Issue with a node labels Jul 18, 2024
michael-radency and others added 3 commits July 19, 2024 11:49
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

From a quick glance, this LGTM.
quite a lot of nice improvements 👏🏽 .

Maybe @elsmr or @ShireenMissi can also review this

@michael-radency
Copy link
Contributor Author

From a quick glance, this LGTM. quite a lot of nice improvements 👏🏽 .

Maybe @elsmr or @ShireenMissi can also review this

thanks @netroy , some changes would need a review from product though

@@ -167,3 +176,96 @@ export const checkResponseModeConfiguration = (context: IWebhookFunctions) => {
);
}
};

export async function validateWebhookAuthentication(
Copy link
Member

Choose a reason for hiding this comment

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

BTW, since this so neatly extracted out now, it might be possible to write unit tests for this. If we can add these tests in the same PR, that's be nice, otherwise I can also try to create a PR with just the tests later 🙏🏽

Copy link
Contributor

@ShireenMissi ShireenMissi left a comment

Choose a reason for hiding this comment

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

Mostly looks good 👍
I have the following comments on multi file select field:

  • it seems the files are cleared every time one clicks choose file.
  • there is no way to select files from multiple locations

Also please add some tests

@michael-radency
Copy link
Contributor Author

  • it seems the files are cleared every time one clicks choose file.
  • there is no way to select files from multiple locations

@ShireenMissi
This is how file selectors usually operate. We could preserve selected files and support appending new ones, but in such a case, we would need a mechanism to clear the input. So, while this could be implemented, it would require custom code and styling, which means an increased scope.

@ShireenMissi
Copy link
Contributor

  • it seems the files are cleared every time one clicks choose file.
  • there is no way to select files from multiple locations

@ShireenMissi This is how file selectors usually operate. We could preserve selected files and support appending new ones, but in such a case, we would need a mechanism to clear the input. So, while this could be implemented, it would require custom code and styling, which means an increased scope.

Thanks @michael-radency we can always improve it in next versions, I am happy with the changes you made to the file field👍

michael-radency and others added 3 commits July 29, 2024 13:49
Co-authored-by: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com>
Copy link
Contributor

@ShireenMissi ShireenMissi left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Copy link
Contributor

✅ All Cypress E2E specs passed

Copy link

cypress bot commented Jul 29, 2024

Passing run #6179 ↗︎

0 389 0 0 Flakiness 0

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 michael-radency 🗃️ e2e/*
Project: n8n Commit: c2c33db04c
Status: Passed Duration: 04:12 💡
Started: Jul 29, 2024 12:47 PM Ended: Jul 29, 2024 12:51 PM

Review all test suite changes for PR #10092 ↗︎

@michael-radency michael-radency merged commit 711b667 into master Jul 29, 2024
27 checks passed
@michael-radency michael-radency deleted the node-1505-form-trigger-improvements-p1 branch July 29, 2024 12:58
MiloradFilipovic added a commit that referenced this pull request Jul 30, 2024
* master:
  docs: Update login url for OpenAI node (no-changelog) (#10222)
  fix(LinkedIn Node): Fix issue with some characters cutting off posts early (#10185)
  fix(Google BigQuery Node): Send timeoutMs in query, pagination support (#10205)
  feat(core): Show Public API key value only once (no-changelog) (#10126)
  refactor(core): Display stack trace in error reporter (no-changelog) (#10225)
  fix(core): Fix missing successful items on continueErrorOutput with multiple outputs (#10218)
  fix(n8n Form Trigger Node): Remove custom attribution option (no-changelog) (#10229)
  feat(n8n Form Trigger Node): Improvements (#10092)
  refactor(core): Port path, host, port, listen_address and protocol config (no-changelog) (#10223)
  docs: Update add options text (no-changelog) (#10049)
  fix(Postgres Node): Option to treat query parameters enclosed in single quotas as text (#10214)
  refactor(core): Decouple server started event from internal hooks (no-changelog) (#10221)
  feat(Shopify Node): Update Shopify API version (#10155)
  fix(editor): Defer `User saved credentials` telemetry event for OAuth credentials (#10215)
  fix(editor): Fix parameter input glitch when there was an error loading remote options (#10209)
cstuncsik pushed a commit that referenced this pull request Jul 31, 2024
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
Co-authored-by: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Jul 31, 2024
@janober
Copy link
Member

janober commented Jul 31, 2024

Got released with n8n@1.53.0

cstuncsik pushed a commit that referenced this pull request Aug 1, 2024
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
Co-authored-by: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
n8n team Authored by the n8n team node/improvement New feature or request node/issue Issue with a node Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants