fix(editor): Send template id as a number in telemetry events #8484

MiloradFilipovic · 2024-01-29T16:15:05Z

Summary

Our event database expects template id in these events as numbers.
Agreed with @nik8n to send strings if, for some reason, id cannot be parsed to number. This will end up as null in DB as a signal that something is wrong.

Related tickets and issues

Fixes ADO-1764

Review / Merge checklist

PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)
Docs updated or follow-up ticket created.
Tests included.

A bug is not considered fixed, unless a test is added to prevent it from happening again.
A feature is not complete without tests.

tomi

Looks good otherwise, but I'd reconsider if the + operator is the best way to convert a string to number. Also I would have imagined the receiving end would be smart enough to convert strings to numbers, but guess not 🤷

tomi · 2024-01-30T08:30:10Z

packages/editor-ui/src/mixins/workflowHelpers.ts

@@ -1094,7 +1094,7 @@ export const workflowHelpers = defineComponent({
 				const templateId = this.$route.query.templateId;
 				if (templateId) {
 					this.$telemetry.track('User saved new workflow from template', {
-						template_id: templateId,
+						template_id: isNaN(+templateId) ? templateId : +templateId,


This is personal opinion, but I feel the + operator for converting something to a number is such an obscure pattern, as it's not very obvious that there's a conversion happening here. Could we perhaps use parseInt or Number instead? They do behave a bit differently, but it shouldn't matter in this case.

Suggested change

template_id: isNaN(+templateId) ? templateId : +templateId,

const templateIdNum = parseInt(templateId, 10)

template_id: isNaN(templateIdNum) ? templateId : templateIdNum,

Thanks @tomi! This makes sense but decided to go with the unary operator here since, when going over this with @nik8n, we decided that we don't want to ignore non-numeric characters in invalid ids such as 123abc and cast them to 123.
Using + these will also be flagged as NaN.
Also, there is a transformation on the rudderstack side that does this check and converts ids to numbers

Gotcha, makes sense 👍

tomi · 2024-01-30T08:33:20Z

packages/editor-ui/src/views/SetupWorkflowFromTemplateView/setupTemplate.store.ts

@@ -207,7 +207,7 @@ export const useSetupTemplateStore = defineStore('setupTemplate', () => {
 			);

 			telemetry.track('User saved new workflow from template', {
-				template_id: templateId.value,
+				template_id: isNaN(+templateId.value) ? templateId : +templateId.value,


cypress · 2024-01-30T08:43:17Z

3 flaky tests on run #3952 ↗︎

0	339	5	0	3

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 MiloradFilipovic 🗃️ e2e/*
Project: n8n	Commit: `a08787b444`
Status: Passed	Duration: 03:30 💡
Started: Jan 30, 2024 8:39 AM	Ended: Jan 30, 2024 8:43 AM

5-ndv.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
NDV > should not retrieve remote options when required params throw errors		`Test Replay` `Screenshots` `Video`

19-execution.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
Execution > should test manual workflow stop		`Test Replay` `Screenshots` `Video`

32-node-io-filter.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
Node IO Filter > should filter input/output data separately		`Test Replay` `Screenshots` `Video`

Review all test suite changes for PR #8484 ↗︎

github-actions · 2024-01-30T08:43:24Z

✅ All Cypress E2E specs passed

…error-states * master: fix(editor): Send template id as a number in telemetry events (#8484) refactor(core): Replace promisify-d node calls with native promises (no-changelog) (#8464) fix(core): Fix stopping and retrying failed executions (#8480) feat: Add model parameter to OpenAI embeddings (#8481) fix(editor): Disable expression editor modal opening on readonly field (#8457) fix(core): Prevent calling internal hook email event if emailing is disabled (#8462)

janober · 2024-02-02T13:24:22Z

Got released with n8n@1.27.2

* refactor(core): Replace promisify-d node calls with native promises (no-changelog) (n8n-io#8464) * fix(editor): Send template id as a number in telemetry events (n8n-io#8484) * refactor(editor): Prevent router.replace from computed property (no-changelog) (n8n-io#8489) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * feat(core): Remove `own` execution-process mode (n8n-io#8490) * feat(editor): Implement loading and error states for dynamically loaded components in node parameter list (n8n-io#8477) * fix(AwsS3 Node): Fix handling of bucket with dot in name (n8n-io#8475) * refactor(core): Modernize credentials controllers and services (no-changelog) (n8n-io#8488) Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in> * fix(core): Don't report executions that have been paused as failed to rudderstack and log streams (n8n-io#8501) * fix: Properly iterate over credentials with expressions (n8n-io#8502) * refactor(core): Move all code related to `onServerStarted` into `InternalHooks` (no-changelog) (n8n-io#8500) * feat(editor): Send template id as string in all telemetry events (n8n-io#8498) * fix(core): Handle possibly invalid `updatedAt` timestamps in source-control (n8n-io#8485) * fix(core): Forward authorization header when on same domain (n8n-io#8507) * fix(core): Improve handling of wrapped errors (n8n-io#8510) * 🚀 Release 1.27.0 (n8n-io#8512) Co-authored-by: ivov <ivov@users.noreply.github.com> * ci: Skip running `postInstall` scripts for all packages except sqlite3 (no-changelog) (n8n-io#8514) * ci: Fix DB tests (no-changelog) (n8n-io#8513) * fix(core): Fix new graceful shutdown env being always overridden by deprecated env (n8n-io#8503) * fix(HTTP Request Node): Support form data when using pagination (n8n-io#8497) * fix(HTTP Request Node): Require parameter with filled name and value to avoid infinite loop (n8n-io#8454) Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com> Co-authored-by: Elias Meire <elias@meire.dev> * fix(Microsoft Excel 365 Node): Upsert append new rows at the end of used range, option to append at the end of selected range (n8n-io#8461) * fix(Slack Node): Attachments fix (n8n-io#8471) Co-authored-by: Elias Meire <elias@meire.dev> * feat: Azure Open AI chat model & embeddings (n8n-io#8522) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * fix(core): Fix test runs of triggers that rely on static data (n8n-io#8524) * ci: Replace `pnpm/action-setup` action with corepack (no-changelog) (n8n-io#8504) * fix(MongoDB Node): Fix "Maximum call stack size exceeded" error on too many rows (n8n-io#8530) * fix: Allow Date/Luxon objects and additional formats in DateTime validation (n8n-io#8525) * fix(editor): Prune values that are not in the schema in the ResourceMapper component (n8n-io#8478) * fix(core): Fix PermissionChecker.check, and add additional unit tests (n8n-io#8528) * fix(core): Fix DropRoleMapping migration (n8n-io#8521) * fix(core): Ensure AxiosError status always gets copied over to NodeApiError (n8n-io#8509) * fix(Embeddings OpenAI Node): Fix dynamic models fetching (n8n-io#8533) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * fix(core): Fix workflow tagging failure due to unique constraint check (n8n-io#8505) * ci: Fix release-create-pr.yml (no-changelog) * fix(core): Upgrade nodemailer to address an exploit (n8n-io#8535) * feat(RabbitMQ Trigger Node): Add options to configure assert of exchanges and queues (n8n-io#8430) * feat(editor): Add delete and disable button to nodes on hover (n8n-io#8482) * feat(Email Trigger (IMAP) Node): Upgrade mailparser (n8n-io#8539) * refactor(core): Streamline flows in multi-main mode (no-changelog) (n8n-io#8446) * fix: Remove ts-node from overrides and typeorm script (no-changelog) (n8n-io#8547) * fix: Update BaseChatModel import checks for MistralAI compatibility (n8n-io#8527) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * docs: Add encryption key check to breaking changes list (n8n-io#8551) * refactor(core): Lock `webhook` process out of multi-main setup (no-changelog) (n8n-io#8549) * refactor(core): Continue breaking dependency cycles (no-changelog) (n8n-io#8545) * fix(core): Use trx manager instead of repository for tags overwrite (n8n-io#8557) * build: Fix outdated import to fix build (no-changelog) (n8n-io#8558) * refactor(core): Couple of refactors on WorkflowRunner and ActiveExecutions (no-changelog) (n8n-io#8487) * feat: Add assignment component with drag and drop to Set node (n8n-io#8283) Co-authored-by: Giulio Andreini <andreini@netseven.it> * ci: Update validate-n8n-pull-request-title action (no-changelog) (n8n-io#8553) * fix(core): Use hostname from URL instead of Host header for SNI (n8n-io#8562) * fix(Microsoft Outlook Node): Download executes more than once per incoming item (n8n-io#8566) * 🚀 Release 1.28.0 (n8n-io#8569) Co-authored-by: ivov <ivov@users.noreply.github.com> * ci(core): Avoid slow bcrypt calls in tests (no-changelog) (n8n-io#8570) * fix(core): Upgrade rudderstack sdk to address npm postInstall issues (n8n-io#8568) * feat: Upgrade typeorm, sqlite3, and pg/pg-promise (n8n-io#8579) * build: Add GitHub issue form for reporting bugs (n8n-io#8585) * fix(Google Sheets Trigger Node): First non-header row is ignored when using on row added event (n8n-io#8580) * fix(RSS Feed Trigger Node): Save last item's date instead of last execution date (n8n-io#8572) * feat(core): Migrate to n8n's typeorm fork (n8n-io#8590) * refactor: Add lint rule for unsafe property access with lodash get/set (no-changelog) (n8n-io#8587) * fix(HTTP Request Node): Errorneous binary object without content-disposition response header (n8n-io#8583) Co-authored-by: Marcus <marcus@n8n.io> * fix(core): Custom workflow tool tweaks (n8n-io#8561) * Fixes imports and enables workers view --------- Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com> Co-authored-by: Milorad FIlipović <milorad@n8n.io> Co-authored-by: oleg <me@olegivaniv.com> Co-authored-by: Iván Ovejero <ivov.src@gmail.com> Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in> Co-authored-by: Danny Martini <despair.blue@gmail.com> Co-authored-by: Omar Ajoue <krynble@gmail.com> Co-authored-by: Elias Meire <elias@meire.dev> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: ivov <ivov@users.noreply.github.com> Co-authored-by: Michael Kret <88898367+michael-radency@users.noreply.github.com> Co-authored-by: Andrea Ascari <dev.ascariandrea@gmail.com> Co-authored-by: Giulio Andreini <andreini@netseven.it> Co-authored-by: Cornelius Suermann <cornelius@n8n.io> Co-authored-by: Bruno Inec <contact@sweenu.xyz> Co-authored-by: Marcus <marcus@n8n.io>

* Andy/joy 277 mergeupdate n8n 128 (#54) * refactor(core): Replace promisify-d node calls with native promises (no-changelog) (n8n-io#8464) * fix(editor): Send template id as a number in telemetry events (n8n-io#8484) * refactor(editor): Prevent router.replace from computed property (no-changelog) (n8n-io#8489) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * feat(core): Remove `own` execution-process mode (n8n-io#8490) * feat(editor): Implement loading and error states for dynamically loaded components in node parameter list (n8n-io#8477) * fix(AwsS3 Node): Fix handling of bucket with dot in name (n8n-io#8475) * refactor(core): Modernize credentials controllers and services (no-changelog) (n8n-io#8488) Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in> * fix(core): Don't report executions that have been paused as failed to rudderstack and log streams (n8n-io#8501) * fix: Properly iterate over credentials with expressions (n8n-io#8502) * refactor(core): Move all code related to `onServerStarted` into `InternalHooks` (no-changelog) (n8n-io#8500) * feat(editor): Send template id as string in all telemetry events (n8n-io#8498) * fix(core): Handle possibly invalid `updatedAt` timestamps in source-control (n8n-io#8485) * fix(core): Forward authorization header when on same domain (n8n-io#8507) * fix(core): Improve handling of wrapped errors (n8n-io#8510) * 🚀 Release 1.27.0 (n8n-io#8512) Co-authored-by: ivov <ivov@users.noreply.github.com> * ci: Skip running `postInstall` scripts for all packages except sqlite3 (no-changelog) (n8n-io#8514) * ci: Fix DB tests (no-changelog) (n8n-io#8513) * fix(core): Fix new graceful shutdown env being always overridden by deprecated env (n8n-io#8503) * fix(HTTP Request Node): Support form data when using pagination (n8n-io#8497) * fix(HTTP Request Node): Require parameter with filled name and value to avoid infinite loop (n8n-io#8454) Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com> Co-authored-by: Elias Meire <elias@meire.dev> * fix(Microsoft Excel 365 Node): Upsert append new rows at the end of used range, option to append at the end of selected range (n8n-io#8461) * fix(Slack Node): Attachments fix (n8n-io#8471) Co-authored-by: Elias Meire <elias@meire.dev> * feat: Azure Open AI chat model & embeddings (n8n-io#8522) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * fix(core): Fix test runs of triggers that rely on static data (n8n-io#8524) * ci: Replace `pnpm/action-setup` action with corepack (no-changelog) (n8n-io#8504) * fix(MongoDB Node): Fix "Maximum call stack size exceeded" error on too many rows (n8n-io#8530) * fix: Allow Date/Luxon objects and additional formats in DateTime validation (n8n-io#8525) * fix(editor): Prune values that are not in the schema in the ResourceMapper component (n8n-io#8478) * fix(core): Fix PermissionChecker.check, and add additional unit tests (n8n-io#8528) * fix(core): Fix DropRoleMapping migration (n8n-io#8521) * fix(core): Ensure AxiosError status always gets copied over to NodeApiError (n8n-io#8509) * fix(Embeddings OpenAI Node): Fix dynamic models fetching (n8n-io#8533) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * fix(core): Fix workflow tagging failure due to unique constraint check (n8n-io#8505) * ci: Fix release-create-pr.yml (no-changelog) * fix(core): Upgrade nodemailer to address an exploit (n8n-io#8535) * feat(RabbitMQ Trigger Node): Add options to configure assert of exchanges and queues (n8n-io#8430) * feat(editor): Add delete and disable button to nodes on hover (n8n-io#8482) * feat(Email Trigger (IMAP) Node): Upgrade mailparser (n8n-io#8539) * refactor(core): Streamline flows in multi-main mode (no-changelog) (n8n-io#8446) * fix: Remove ts-node from overrides and typeorm script (no-changelog) (n8n-io#8547) * fix: Update BaseChatModel import checks for MistralAI compatibility (n8n-io#8527) Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> * docs: Add encryption key check to breaking changes list (n8n-io#8551) * refactor(core): Lock `webhook` process out of multi-main setup (no-changelog) (n8n-io#8549) * refactor(core): Continue breaking dependency cycles (no-changelog) (n8n-io#8545) * fix(core): Use trx manager instead of repository for tags overwrite (n8n-io#8557) * build: Fix outdated import to fix build (no-changelog) (n8n-io#8558) * refactor(core): Couple of refactors on WorkflowRunner and ActiveExecutions (no-changelog) (n8n-io#8487) * feat: Add assignment component with drag and drop to Set node (n8n-io#8283) Co-authored-by: Giulio Andreini <andreini@netseven.it> * ci: Update validate-n8n-pull-request-title action (no-changelog) (n8n-io#8553) * fix(core): Use hostname from URL instead of Host header for SNI (n8n-io#8562) * fix(Microsoft Outlook Node): Download executes more than once per incoming item (n8n-io#8566) * 🚀 Release 1.28.0 (n8n-io#8569) Co-authored-by: ivov <ivov@users.noreply.github.com> * ci(core): Avoid slow bcrypt calls in tests (no-changelog) (n8n-io#8570) * fix(core): Upgrade rudderstack sdk to address npm postInstall issues (n8n-io#8568) * feat: Upgrade typeorm, sqlite3, and pg/pg-promise (n8n-io#8579) * build: Add GitHub issue form for reporting bugs (n8n-io#8585) * fix(Google Sheets Trigger Node): First non-header row is ignored when using on row added event (n8n-io#8580) * fix(RSS Feed Trigger Node): Save last item's date instead of last execution date (n8n-io#8572) * feat(core): Migrate to n8n's typeorm fork (n8n-io#8590) * refactor: Add lint rule for unsafe property access with lodash get/set (no-changelog) (n8n-io#8587) * fix(HTTP Request Node): Errorneous binary object without content-disposition response header (n8n-io#8583) Co-authored-by: Marcus <marcus@n8n.io> * fix(core): Custom workflow tool tweaks (n8n-io#8561) * Fixes imports and enables workers view --------- Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com> Co-authored-by: Milorad FIlipović <milorad@n8n.io> Co-authored-by: oleg <me@olegivaniv.com> Co-authored-by: Iván Ovejero <ivov.src@gmail.com> Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in> Co-authored-by: Danny Martini <despair.blue@gmail.com> Co-authored-by: Omar Ajoue <krynble@gmail.com> Co-authored-by: Elias Meire <elias@meire.dev> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: ivov <ivov@users.noreply.github.com> Co-authored-by: Michael Kret <88898367+michael-radency@users.noreply.github.com> Co-authored-by: Andrea Ascari <dev.ascariandrea@gmail.com> Co-authored-by: Giulio Andreini <andreini@netseven.it> Co-authored-by: Cornelius Suermann <cornelius@n8n.io> Co-authored-by: Bruno Inec <contact@sweenu.xyz> Co-authored-by: Marcus <marcus@n8n.io> * Enables advanced permissions * Fixes workflow statistics update errors --------- Signed-off-by: Oleg Ivaniv <me@olegivaniv.com> Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com> Co-authored-by: Milorad FIlipović <milorad@n8n.io> Co-authored-by: oleg <me@olegivaniv.com> Co-authored-by: Iván Ovejero <ivov.src@gmail.com> Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in> Co-authored-by: Danny Martini <despair.blue@gmail.com> Co-authored-by: Omar Ajoue <krynble@gmail.com> Co-authored-by: Elias Meire <elias@meire.dev> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: ivov <ivov@users.noreply.github.com> Co-authored-by: Michael Kret <88898367+michael-radency@users.noreply.github.com> Co-authored-by: Andrea Ascari <dev.ascariandrea@gmail.com> Co-authored-by: Giulio Andreini <andreini@netseven.it> Co-authored-by: Cornelius Suermann <cornelius@n8n.io> Co-authored-by: Bruno Inec <contact@sweenu.xyz> Co-authored-by: Marcus <marcus@n8n.io>

fix(editor): Send template id as a number in telemetry events

a08787b

MiloradFilipovic self-assigned this Jan 29, 2024

n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Jan 29, 2024

tomi approved these changes Jan 30, 2024

View reviewed changes

MiloradFilipovic merged commit 327cc8d into master Jan 30, 2024
30 checks passed

MiloradFilipovic deleted the ADO-1764-send-template-id-as-number branch January 30, 2024 09:37

MiloradFilipovic mentioned this pull request Jan 30, 2024

feat(editor): Send template id as string in all telemetry events #8498

Merged

3 tasks

github-actions bot mentioned this pull request Jan 31, 2024

🚀 Release 1.27.0 #8512

Merged

janober added the Released label Feb 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(editor): Send template id as a number in telemetry events #8484

fix(editor): Send template id as a number in telemetry events #8484

MiloradFilipovic commented Jan 29, 2024

tomi left a comment

tomi Jan 30, 2024

MiloradFilipovic Jan 30, 2024 •

edited

Loading

tomi Jan 30, 2024

tomi Jan 30, 2024

cypress bot commented Jan 30, 2024

github-actions bot commented Jan 30, 2024

janober commented Feb 2, 2024

	template_id: isNaN(+templateId) ? templateId : +templateId,
	const templateIdNum = parseInt(templateId, 10)
	template_id: isNaN(templateIdNum) ? templateId : templateIdNum,

fix(editor): Send template id as a number in telemetry events #8484

fix(editor): Send template id as a number in telemetry events #8484

Conversation

MiloradFilipovic commented Jan 29, 2024

Summary

Related tickets and issues

Review / Merge checklist

tomi left a comment

Choose a reason for hiding this comment

tomi Jan 30, 2024

Choose a reason for hiding this comment

MiloradFilipovic Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

tomi Jan 30, 2024

Choose a reason for hiding this comment

tomi Jan 30, 2024

Choose a reason for hiding this comment

cypress bot commented Jan 30, 2024

3 flaky tests on run #3952 ↗︎

Review all test suite changes for PR #8484 ↗︎

github-actions bot commented Jan 30, 2024

janober commented Feb 2, 2024

MiloradFilipovic Jan 30, 2024 •

edited

Loading