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

fix(Gmail Trigger Node): Don't return date instances, but date strings instead #10582

Conversation

despairblue
Copy link
Contributor

@despairblue despairblue commented Aug 28, 2024

Summary

The Gmail Trigger node uses a library to parse emails and that library parses the date header into an instance of Date. Data Objects don't support complex types, e.g. classes, they only support basic data types, like numbers and strings.

Returning a complex data type led to weird behaviour down the line in the workflow. E.g. passing the date instance through a set node would stringify it multiple times and you'd end up with a date string with double quotes.

This PR turns the date instance into a ISO date string before passing it along to other nodes.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/PAY-1810/date-string-contains-double-quotes-at-the-start-and-the-end-when

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)

…s instead

The Gmail Trigger node uses a library to parse emails and that library
parses the date header into an instance of Date. Data Objects don't
support complex types, e.g. classes, they only support basic data types,
like numbers and strings.

Returning a complex data type led to weird behaviour down the line in
the workflow. E.g. passing the date instance through a set node would
stringify it multiple times and you'd end up with a date string with
double quotes.

This PR turns the date instance into a ISO date string before passing it
along to other nodes.
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team node/improvement New feature or request labels Aug 28, 2024
@despairblue despairblue marked this pull request as ready for review August 28, 2024 09:20
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.

Thanks for fixing this 🙏

Copy link
Contributor

✅ All Cypress E2E specs passed

Copy link

cypress bot commented Aug 28, 2024

n8n    Run #6646

Run Properties:  status check passed Passed #6646  •  git commit 1cb51ce857: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Project n8n
Branch Review pay-1810-date-string-contains-double-quotes-at-the-start-and-the-end
Run status status check passed Passed #6646
Run duration 04m 27s
Commit git commit 1cb51ce857: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Committer Danny Martini
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 419
View all changes introduced in this branch ↗︎

@despairblue despairblue merged commit 9e1dac0 into master Aug 28, 2024
34 checks passed
@despairblue despairblue deleted the pay-1810-date-string-contains-double-quotes-at-the-start-and-the-end branch August 28, 2024 13:43
MiloradFilipovic added a commit that referenced this pull request Aug 28, 2024
* master: (24 commits)
  feat(core): Switch to MJML for email templates (#10518)
  fix(Gmail Trigger Node): Don't return date instances, but date strings instead (#10582)
  fix(core): Deduplicate sentry events using error stacktraces instead (no-changelog) (#10590)
  feat(editor): Implement new app layout (#10548)
  refactor(core): Standardize filename casing for services and Public API (no-changelog) (#10579)
  🚀 Release 1.57.0 (#10587)
  fix(editor): Add tooltips to workflow history button (#10570)
  ci: Fix provenance generation during NPM publish (no-changelog) (#10586)
  feat(core): Expose queue metrics for Prometheus (#10559)
  refactor(core): Map out pubsub messages (no-changelog) (#10566)
  fix: Fix scenario prefix not being passed (no-changelog) (#10575)
  refactor(core): Convert `verbose` to `debug` logs (#10574)
  refactor(core): Use `@/databases/` instead of `@db/` (no-changelog) (#10573)
  ci: Fix destroy benchmark env workflow (no-changelog) (#10572)
  feat: Add benchmarking of pooled sqlite (no-changelog) (#10550)
  refactor(editor): User journey link to n8n.io (#10331)
  fix(Wait Node): Prevent waiting until invalid date (#10523)
  refactor(core): Standardize filename casing for controllers and databases (no-changelog) (#10564)
  refactor(core): Allow custom types on getCredentials (no-changelog) (#10567)
  fix(editor): Scale output item selector input width with value (#10555)
  ...

# Conflicts:
#	packages/editor-ui/src/stores/assistant.store.ts
@github-actions github-actions bot mentioned this pull request Sep 5, 2024
@janober
Copy link
Member

janober commented Sep 5, 2024

Got released with n8n@1.58.0

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 Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants