From 589bafead313768ee4302b0d80903ec25ced284c Mon Sep 17 00:00:00 2001 From: Natay Aberra Date: Fri, 20 Dec 2024 12:38:33 -0500 Subject: [PATCH 1/7] PDE-5636 update --- packages/cli/src/oclif/ZapierBaseCommand.js | 2 +- packages/cli/src/utils/analytics.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/oclif/ZapierBaseCommand.js b/packages/cli/src/oclif/ZapierBaseCommand.js index cae912ab1..67eb277c3 100644 --- a/packages/cli/src/oclif/ZapierBaseCommand.js +++ b/packages/cli/src/oclif/ZapierBaseCommand.js @@ -346,7 +346,7 @@ class ZapierBaseCommand extends Command { if (!this.args) { throw new Error('unable to record analytics until args are parsed'); } - return recordAnalytics(this.id, true, Object.keys(this.args), this.flags); + return recordAnalytics(this.id, true, Object.values(this.args), this.flags); } } diff --git a/packages/cli/src/utils/analytics.js b/packages/cli/src/utils/analytics.js index 9fc4d99bd..9f1ddb3e9 100644 --- a/packages/cli/src/utils/analytics.js +++ b/packages/cli/src/utils/analytics.js @@ -2,6 +2,7 @@ const { callAPI } = require('./api'); // const { readFile } = require('./files'); const debug = require('debug')('zapier:analytics'); const pkg = require('../../package.json'); +const { getLinkedAppConfig } = require('../utils/api'); const { ANALYTICS_KEY, ANALYTICS_MODES, IS_TESTING } = require('../constants'); const { readUserConfig, writeUserConfig } = require('./userConfig'); @@ -20,24 +21,26 @@ const shouldSkipAnalytics = (mode) => process.env.DISABLE_ZAPIER_ANALYTICS || mode === ANALYTICS_MODES.disabled; -const recordAnalytics = async (command, isValidCommand, argNames, flags) => { +const recordAnalytics = async (command, isValidCommand, argValues, flags) => { const analyticsMode = await currentAnalyticsMode(); if (shouldSkipAnalytics(analyticsMode)) { debug('skipping analytics'); return; } - const shouldRecordAnonymously = analyticsMode === ANALYTICS_MODES.anonymous; - + // We don't want to "explode" if appID is missing + const linkedAppId = (await getLinkedAppConfig(undefined, false))?.id; // to make this more testable, we should split this out into its own function const analyticsBody = { command, isValidCommand, - numArgs: argNames.length, + numArgs: argValues.length, + arguments: argValues, + app_id: linkedAppId, flags: { ...flags, - ...(command === 'help' ? { helpCommand: argNames[0] } : {}), // include the beginning of args so we know what they want help on + ...(command === 'help' ? { helpCommand: argValues[0] } : {}), // include the beginning of args so we know what they want help on }, cliVersion: pkg.version, os: shouldRecordAnonymously ? undefined : process.platform, From b37ea5b6d177f14858fe17a2afd3c7b4fa7ee08b Mon Sep 17 00:00:00 2001 From: Natay Aberra Date: Fri, 20 Dec 2024 12:38:33 -0500 Subject: [PATCH 2/7] PDE-5636 update --- packages/cli/src/oclif/ZapierBaseCommand.js | 2 +- packages/cli/src/utils/analytics.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/oclif/ZapierBaseCommand.js b/packages/cli/src/oclif/ZapierBaseCommand.js index cae912ab1..67eb277c3 100644 --- a/packages/cli/src/oclif/ZapierBaseCommand.js +++ b/packages/cli/src/oclif/ZapierBaseCommand.js @@ -346,7 +346,7 @@ class ZapierBaseCommand extends Command { if (!this.args) { throw new Error('unable to record analytics until args are parsed'); } - return recordAnalytics(this.id, true, Object.keys(this.args), this.flags); + return recordAnalytics(this.id, true, Object.values(this.args), this.flags); } } diff --git a/packages/cli/src/utils/analytics.js b/packages/cli/src/utils/analytics.js index 9fc4d99bd..9f1ddb3e9 100644 --- a/packages/cli/src/utils/analytics.js +++ b/packages/cli/src/utils/analytics.js @@ -2,6 +2,7 @@ const { callAPI } = require('./api'); // const { readFile } = require('./files'); const debug = require('debug')('zapier:analytics'); const pkg = require('../../package.json'); +const { getLinkedAppConfig } = require('../utils/api'); const { ANALYTICS_KEY, ANALYTICS_MODES, IS_TESTING } = require('../constants'); const { readUserConfig, writeUserConfig } = require('./userConfig'); @@ -20,24 +21,26 @@ const shouldSkipAnalytics = (mode) => process.env.DISABLE_ZAPIER_ANALYTICS || mode === ANALYTICS_MODES.disabled; -const recordAnalytics = async (command, isValidCommand, argNames, flags) => { +const recordAnalytics = async (command, isValidCommand, argValues, flags) => { const analyticsMode = await currentAnalyticsMode(); if (shouldSkipAnalytics(analyticsMode)) { debug('skipping analytics'); return; } - const shouldRecordAnonymously = analyticsMode === ANALYTICS_MODES.anonymous; - + // We don't want to "explode" if appID is missing + const linkedAppId = (await getLinkedAppConfig(undefined, false))?.id; // to make this more testable, we should split this out into its own function const analyticsBody = { command, isValidCommand, - numArgs: argNames.length, + numArgs: argValues.length, + arguments: argValues, + app_id: linkedAppId, flags: { ...flags, - ...(command === 'help' ? { helpCommand: argNames[0] } : {}), // include the beginning of args so we know what they want help on + ...(command === 'help' ? { helpCommand: argValues[0] } : {}), // include the beginning of args so we know what they want help on }, cliVersion: pkg.version, os: shouldRecordAnonymously ? undefined : process.platform, From 401f2adc87b1a5b317f1ec898a6f1f20373e1829 Mon Sep 17 00:00:00 2001 From: Natay Aberra Date: Mon, 23 Dec 2024 09:46:52 -0500 Subject: [PATCH 3/7] PDE-5636 remove arguments from recording --- packages/cli/src/oclif/ZapierBaseCommand.js | 2 +- packages/cli/src/utils/analytics.js | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/oclif/ZapierBaseCommand.js b/packages/cli/src/oclif/ZapierBaseCommand.js index 67eb277c3..cae912ab1 100644 --- a/packages/cli/src/oclif/ZapierBaseCommand.js +++ b/packages/cli/src/oclif/ZapierBaseCommand.js @@ -346,7 +346,7 @@ class ZapierBaseCommand extends Command { if (!this.args) { throw new Error('unable to record analytics until args are parsed'); } - return recordAnalytics(this.id, true, Object.values(this.args), this.flags); + return recordAnalytics(this.id, true, Object.keys(this.args), this.flags); } } diff --git a/packages/cli/src/utils/analytics.js b/packages/cli/src/utils/analytics.js index 9f1ddb3e9..0eadd706b 100644 --- a/packages/cli/src/utils/analytics.js +++ b/packages/cli/src/utils/analytics.js @@ -21,7 +21,7 @@ const shouldSkipAnalytics = (mode) => process.env.DISABLE_ZAPIER_ANALYTICS || mode === ANALYTICS_MODES.disabled; -const recordAnalytics = async (command, isValidCommand, argValues, flags) => { +const recordAnalytics = async (command, isValidCommand, argNames, flags) => { const analyticsMode = await currentAnalyticsMode(); if (shouldSkipAnalytics(analyticsMode)) { @@ -35,12 +35,11 @@ const recordAnalytics = async (command, isValidCommand, argValues, flags) => { const analyticsBody = { command, isValidCommand, - numArgs: argValues.length, - arguments: argValues, + numArgs: argNames.length, app_id: linkedAppId, flags: { ...flags, - ...(command === 'help' ? { helpCommand: argValues[0] } : {}), // include the beginning of args so we know what they want help on + ...(command === 'help' ? { helpCommand: argNames[0] } : {}), // include the beginning of args so we know what they want help on }, cliVersion: pkg.version, os: shouldRecordAnonymously ? undefined : process.platform, From e88c0c1def1c92d8111d397ca4ff3d289b0f4fdf Mon Sep 17 00:00:00 2001 From: Natay Aberra Date: Mon, 23 Dec 2024 10:29:37 -0500 Subject: [PATCH 4/7] PDE-5636 consider args only when parsing app ids --- packages/cli/src/oclif/ZapierBaseCommand.js | 2 +- packages/cli/src/utils/analytics.js | 23 ++++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/oclif/ZapierBaseCommand.js b/packages/cli/src/oclif/ZapierBaseCommand.js index cae912ab1..24c23cbfa 100644 --- a/packages/cli/src/oclif/ZapierBaseCommand.js +++ b/packages/cli/src/oclif/ZapierBaseCommand.js @@ -346,7 +346,7 @@ class ZapierBaseCommand extends Command { if (!this.args) { throw new Error('unable to record analytics until args are parsed'); } - return recordAnalytics(this.id, true, Object.keys(this.args), this.flags); + return recordAnalytics(this.id, true, this.args, this.flags); } } diff --git a/packages/cli/src/utils/analytics.js b/packages/cli/src/utils/analytics.js index 0eadd706b..771a34c68 100644 --- a/packages/cli/src/utils/analytics.js +++ b/packages/cli/src/utils/analytics.js @@ -21,25 +21,38 @@ const shouldSkipAnalytics = (mode) => process.env.DISABLE_ZAPIER_ANALYTICS || mode === ANALYTICS_MODES.disabled; -const recordAnalytics = async (command, isValidCommand, argNames, flags) => { +const recordAnalytics = async (command, isValidCommand, args, flags) => { const analyticsMode = await currentAnalyticsMode(); if (shouldSkipAnalytics(analyticsMode)) { debug('skipping analytics'); return; } + const argKeys = Object.keys(args); + const shouldRecordAnonymously = analyticsMode === ANALYTICS_MODES.anonymous; - // We don't want to "explode" if appID is missing - const linkedAppId = (await getLinkedAppConfig(undefined, false))?.id; + + const integrationIDKey = argKeys.find( + (key) => key.toLowerCase() === 'integrationid', + ); + const integrationID = integrationIDKey ? args[integrationIDKey] : undefined; + + // Some commands ( like "zapier convert" ) won't have an app directory when called. + // Instead, having the app ID in the arguments. + // Fallback to using "integrationid" in arguments ( if it's there ) + // and don't want to "explode" if appID is missing + const linkedAppId = + (await getLinkedAppConfig(undefined, false))?.id || integrationID; + // to make this more testable, we should split this out into its own function const analyticsBody = { command, isValidCommand, - numArgs: argNames.length, + numArgs: argKeys.length, app_id: linkedAppId, flags: { ...flags, - ...(command === 'help' ? { helpCommand: argNames[0] } : {}), // include the beginning of args so we know what they want help on + ...(command === 'help' ? { helpCommand: argKeys[0] } : {}), // include the beginning of args so we know what they want help on }, cliVersion: pkg.version, os: shouldRecordAnonymously ? undefined : process.platform, From a0109d001921762f725918c1aeb01d3712e9b3a7 Mon Sep 17 00:00:00 2001 From: Natay Aberra Date: Mon, 23 Dec 2024 10:42:14 -0500 Subject: [PATCH 5/7] PDE-5636 update comment --- packages/cli/src/utils/analytics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/utils/analytics.js b/packages/cli/src/utils/analytics.js index 771a34c68..1c9779067 100644 --- a/packages/cli/src/utils/analytics.js +++ b/packages/cli/src/utils/analytics.js @@ -39,7 +39,7 @@ const recordAnalytics = async (command, isValidCommand, args, flags) => { // Some commands ( like "zapier convert" ) won't have an app directory when called. // Instead, having the app ID in the arguments. - // Fallback to using "integrationid" in arguments ( if it's there ) + // In this case, we fallback to using "integrationid" in arguments ( if it's there ) // and don't want to "explode" if appID is missing const linkedAppId = (await getLinkedAppConfig(undefined, false))?.id || integrationID; From 81080ae0e2d3cbd99c7f8fea3239bc85716080f7 Mon Sep 17 00:00:00 2001 From: Natay Aberra Date: Fri, 27 Dec 2024 11:34:15 -0500 Subject: [PATCH 6/7] PDE-5636 use response url as fallback in debug logs --- packages/cli/src/utils/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/utils/api.js b/packages/cli/src/utils/api.js index 5b7287a63..a0691b52f 100644 --- a/packages/cli/src/utils/api.js +++ b/packages/cli/src/utils/api.js @@ -95,7 +95,7 @@ const callAPI = async ( } } - debug(`>> ${requestOptions.method} ${requestOptions.url}`); + debug(`>> ${requestOptions.method} ${requestOptions.url || res.url}`); if (requestOptions.body) { const replacementStr = 'raw zip removed in logs'; From 89f4c4561275ed7d8485b8dd131c844acc39de31 Mon Sep 17 00:00:00 2001 From: Natay Aberra Date: Mon, 30 Dec 2024 09:34:53 -0500 Subject: [PATCH 7/7] make appId argument camel case --- packages/cli/src/utils/analytics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/utils/analytics.js b/packages/cli/src/utils/analytics.js index 1c9779067..79f0c50d5 100644 --- a/packages/cli/src/utils/analytics.js +++ b/packages/cli/src/utils/analytics.js @@ -49,7 +49,7 @@ const recordAnalytics = async (command, isValidCommand, args, flags) => { command, isValidCommand, numArgs: argKeys.length, - app_id: linkedAppId, + appId: linkedAppId, flags: { ...flags, ...(command === 'help' ? { helpCommand: argKeys[0] } : {}), // include the beginning of args so we know what they want help on