From 7f2364e41167611c41003559de65cee1fece5464 Mon Sep 17 00:00:00 2001 From: Utsab Chowdhury Date: Mon, 4 Mar 2024 10:29:47 +0530 Subject: [PATCH 1/4] fix: amplitude fix for user operations --- src/v0/destinations/am/transform.js | 62 ++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/src/v0/destinations/am/transform.js b/src/v0/destinations/am/transform.js index afd72b77e1..126750aefe 100644 --- a/src/v0/destinations/am/transform.js +++ b/src/v0/destinations/am/transform.js @@ -268,7 +268,7 @@ const updateConfigProperty = (message, payload, mappingJson, validatePayload, Co } }); }; -const identifyBuilder = (message, destination, rawPayload) => { +const userPropertiesHandler = (message, destination, rawPayload) => { // update payload user_properties from userProperties/traits/context.traits/nested traits of Rudder message // traits like address converted to top level user properties (think we can skip this extra processing as AM supports nesting upto 40 levels) let traits = getFieldValueFromMessage(message, 'traits'); @@ -335,6 +335,57 @@ const getDefaultResponseData = (message, rawPayload, evType, groupInfo) => { const groups = groupInfo && cloneDeep(groupInfo); return { groups, rawPayload }; }; + +const userPropertiesPostProcess = (rawPayload) => { + const operationList = [ + '$setOnce', + '$add', + '$unset', + '$append', + '$prepend', + '$preInsert', + '$postInsert', + '$remove', + ]; + // eslint-disable-next-line @typescript-eslint/naming-convention + const { user_properties } = rawPayload; + const userPropertiesKeys = Object.keys(user_properties).filter( + (key) => !operationList.includes(key), + ); + const duplicatekeys = new Set(); + // eslint-disable-next-line no-restricted-syntax, guard-for-in + for (const key of userPropertiesKeys) { + // check if any of the keys are present in the user_properties $setOnce, $add, $unset, $append, $prepend, $preInsert, $postInsert, $remove keys as well as root level + + if ( + operationList.some( + (operation) => user_properties[operation] && user_properties[operation][key], + ) + ) { + duplicatekeys.add(key); + } + } + // eslint-disable-next-line no-restricted-syntax, guard-for-in + for (const key of duplicatekeys) { + delete user_properties[key]; + } + + const setProps = {}; + // eslint-disable-next-line no-restricted-syntax + for (const [key, value] of Object.entries(user_properties)) { + if (!operationList.includes(key)) { + setProps[key] = value; + delete user_properties[key]; + } + } + + if (Object.keys(setProps).length > 0) { + user_properties.$set = setProps; + } + + rawPayload.user_properties = user_properties; + return rawPayload; +}; const getResponseData = (evType, destination, rawPayload, message, groupInfo) => { let groups; @@ -342,7 +393,7 @@ const getResponseData = (evType, destination, rawPayload, message, groupInfo) => case EventType.IDENTIFY: // event_type for identify event is $identify rawPayload.event_type = IDENTIFY_AM; - rawPayload = identifyBuilder(message, destination, rawPayload); + rawPayload = userPropertiesHandler(message, destination, rawPayload); break; case EventType.GROUP: // event_type for identify event is $identify @@ -357,8 +408,15 @@ const getResponseData = (evType, destination, rawPayload, message, groupInfo) => case EventType.ALIAS: break; default: + if (destination.Config.enableEnhncedUserOpertaions) { + // handle all other events like track, page, screen for user properties + rawPayload = userPropertiesHandler(message, destination, rawPayload); + } ({ groups, rawPayload } = getDefaultResponseData(message, rawPayload, evType, groupInfo)); } + if (destination.Config.enableEnhncedUserOpertaions) { + rawPayload = userPropertiesPostProcess(rawPayload); + } return { rawPayload, groups }; }; From 0d03c67ede387ee2b8cbf491109d32a6f2ee3609 Mon Sep 17 00:00:00 2001 From: Utsab Chowdhury Date: Mon, 4 Mar 2024 12:52:48 +0530 Subject: [PATCH 2/4] chore: clean up and add test cases --- src/v0/destinations/am/transform.js | 55 ++--------------------- src/v0/destinations/am/util.test.js | 69 ++++++++++++++++++++++++++++- src/v0/destinations/am/utils.js | 56 +++++++++++++++++++++++ 3 files changed, 127 insertions(+), 53 deletions(-) diff --git a/src/v0/destinations/am/transform.js b/src/v0/destinations/am/transform.js index 126750aefe..bc08315fa8 100644 --- a/src/v0/destinations/am/transform.js +++ b/src/v0/destinations/am/transform.js @@ -336,56 +336,7 @@ const getDefaultResponseData = (message, rawPayload, evType, groupInfo) => { return { groups, rawPayload }; }; -const userPropertiesPostProcess = (rawPayload) => { - const operationList = [ - '$setOnce', - '$add', - '$unset', - '$append', - '$prepend', - '$preInsert', - '$postInsert', - '$remove', - ]; - // eslint-disable-next-line @typescript-eslint/naming-convention - const { user_properties } = rawPayload; - const userPropertiesKeys = Object.keys(user_properties).filter( - (key) => !operationList.includes(key), - ); - const duplicatekeys = new Set(); - // eslint-disable-next-line no-restricted-syntax, guard-for-in - for (const key of userPropertiesKeys) { - // check if any of the keys are present in the user_properties $setOnce, $add, $unset, $append, $prepend, $preInsert, $postInsert, $remove keys as well as root level - - if ( - operationList.some( - (operation) => user_properties[operation] && user_properties[operation][key], - ) - ) { - duplicatekeys.add(key); - } - } - // eslint-disable-next-line no-restricted-syntax, guard-for-in - for (const key of duplicatekeys) { - delete user_properties[key]; - } - const setProps = {}; - // eslint-disable-next-line no-restricted-syntax - for (const [key, value] of Object.entries(user_properties)) { - if (!operationList.includes(key)) { - setProps[key] = value; - delete user_properties[key]; - } - } - - if (Object.keys(setProps).length > 0) { - user_properties.$set = setProps; - } - - rawPayload.user_properties = user_properties; - return rawPayload; -}; const getResponseData = (evType, destination, rawPayload, message, groupInfo) => { let groups; @@ -408,14 +359,14 @@ const getResponseData = (evType, destination, rawPayload, message, groupInfo) => case EventType.ALIAS: break; default: - if (destination.Config.enableEnhncedUserOpertaions) { + if (destination.Config.enableEnhancedUserOperations) { // handle all other events like track, page, screen for user properties rawPayload = userPropertiesHandler(message, destination, rawPayload); } ({ groups, rawPayload } = getDefaultResponseData(message, rawPayload, evType, groupInfo)); } - if (destination.Config.enableEnhncedUserOpertaions) { - rawPayload = userPropertiesPostProcess(rawPayload); + if (destination.Config.enableEnhancedUserOperations) { + rawPayload = AMUtils.userPropertiesPostProcess(rawPayload); } return { rawPayload, groups }; }; diff --git a/src/v0/destinations/am/util.test.js b/src/v0/destinations/am/util.test.js index fa30a74757..723ff3a302 100644 --- a/src/v0/destinations/am/util.test.js +++ b/src/v0/destinations/am/util.test.js @@ -1,4 +1,4 @@ -const { getUnsetObj, validateEventType } = require('./utils'); +const { getUnsetObj, validateEventType, userPropertiesPostProcess } = require('./utils'); describe('getUnsetObj', () => { it("should return undefined when 'message.integrations.Amplitude.fieldsToUnset' is not array", () => { @@ -97,3 +97,70 @@ describe('validateEventType', () => { ); }); }); + +describe('userPropertiesPostProcess', () => { + // The function correctly removes duplicate keys found in both operation keys and root level keys. + it('should remove duplicate keys from user_properties', () => { + // Arrange + const rawPayload = { + user_properties: { + $setOnce: { + key1: 'value1', + }, + $add: { + key2: 'value2', + }, + key3: 'value3', + key1: 'value4', + }, + }; + + // Act + const result = userPropertiesPostProcess(rawPayload); + + // Assert + expect(result.user_properties).toEqual({ + $setOnce: { + key1: 'value1', + }, + $add: { + key2: 'value2', + }, + $set: { + key3: 'value3', + }, + }); + }); + + // The function correctly moves root level properties to $set operation. + it('should move root level properties to $set operation when they dont belong to any other operation', () => { + // Arrange + const rawPayload = { + user_properties: { + $setOnce: { + key1: 'value1', + }, + $add: { + key2: 'value2', + }, + key3: 'value3', + }, + }; + + // Act + const result = userPropertiesPostProcess(rawPayload); + + // Assert + expect(result.user_properties).toEqual({ + $set: { + key3: 'value3', + }, + $setOnce: { + key1: 'value1', + }, + $add: { + key2: 'value2', + }, + }); + }); +}); diff --git a/src/v0/destinations/am/utils.js b/src/v0/destinations/am/utils.js index ed1b772fca..4d4fd5dc37 100644 --- a/src/v0/destinations/am/utils.js +++ b/src/v0/destinations/am/utils.js @@ -122,6 +122,61 @@ const validateEventType = (evType) => { ); } }; + + +const userPropertiesPostProcess = (rawPayload) => { + const operationList = [ + '$setOnce', + '$add', + '$unset', + '$append', + '$prepend', + '$preInsert', + '$postInsert', + '$remove', + ]; + // eslint-disable-next-line @typescript-eslint/naming-convention + const { user_properties } = rawPayload; + const userPropertiesKeys = Object.keys(user_properties).filter( + (key) => !operationList.includes(key), + ); + const duplicatekeys = new Set(); + // eslint-disable-next-line no-restricted-syntax, guard-for-in + for (const key of userPropertiesKeys) { + // check if any of the keys are present in the user_properties $setOnce, $add, $unset, $append, $prepend, $preInsert, $postInsert, $remove keys as well as root level + + if ( + operationList.some( + (operation) => user_properties[operation] && user_properties[operation][key], + ) + ) { + duplicatekeys.add(key); + } + } + // eslint-disable-next-line no-restricted-syntax, guard-for-in + for (const key of duplicatekeys) { + delete user_properties[key]; + } + + // Moving root level properties that doesn't belong to any operation under $set + const setProps = {}; + // eslint-disable-next-line no-restricted-syntax + for (const [key, value] of Object.entries(user_properties)) { + if (!operationList.includes(key)) { + setProps[key] = value; + delete user_properties[key]; + } + } + + if (Object.keys(setProps).length > 0) { + user_properties.$set = setProps; + } + + // eslint-disable-next-line no-param-reassign + rawPayload.user_properties = user_properties; + return rawPayload; +}; + module.exports = { getOSName, getOSVersion, @@ -132,4 +187,5 @@ module.exports = { getEventId, getUnsetObj, validateEventType, + userPropertiesPostProcess }; From 302cbe8724831908d8e551effa84ad6cb5a89ea1 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Mon, 4 Mar 2024 09:38:56 +0000 Subject: [PATCH 3/4] chore(release): 1.57.1 --- CHANGELOG.md | 8 ++++++++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c526b8051..a043cfb6e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +### [1.57.1](https://github.com/rudderlabs/rudder-transformer/compare/v1.57.0...v1.57.1) (2024-03-04) + + +### Bug Fixes + +* amplitude fix for user operations ([7f2364e](https://github.com/rudderlabs/rudder-transformer/commit/7f2364e41167611c41003559de65cee1fece5464)) +* amplitude fix for user operations ([#3153](https://github.com/rudderlabs/rudder-transformer/issues/3153)) ([31869fb](https://github.com/rudderlabs/rudder-transformer/commit/31869fb114bb141d545de01c56f57b97e5aa54a6)) + ## [1.57.0](https://github.com/rudderlabs/rudder-transformer/compare/v1.56.1...v1.57.0) (2024-02-29) diff --git a/package-lock.json b/package-lock.json index dfed6e2397..13be0a6ceb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "rudder-transformer", - "version": "1.57.0", + "version": "1.57.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "rudder-transformer", - "version": "1.57.0", + "version": "1.57.1", "license": "ISC", "dependencies": { "@amplitude/ua-parser-js": "0.7.24", diff --git a/package.json b/package.json index f241515ffc..dccf41a261 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "rudder-transformer", - "version": "1.57.0", + "version": "1.57.1", "description": "", "homepage": "https://github.com/rudderlabs/rudder-transformer#readme", "bugs": { From d1102a27b56eb105e8b6eb528cb31720edb1c0fa Mon Sep 17 00:00:00 2001 From: Dilip Kola <33080863+koladilip@users.noreply.github.com> Date: Mon, 4 Mar 2024 19:38:27 +0530 Subject: [PATCH 4/4] chore: update prepare-for-staging-deploy.yml --- .github/workflows/prepare-for-staging-deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/prepare-for-staging-deploy.yml b/.github/workflows/prepare-for-staging-deploy.yml index e7df8c43a5..a69cf90c8c 100644 --- a/.github/workflows/prepare-for-staging-deploy.yml +++ b/.github/workflows/prepare-for-staging-deploy.yml @@ -101,7 +101,7 @@ jobs: cd rudder-devops BRANCH_NAME="shared-transformer-$TAG_NAME" echo $BRANCH_NAME - if [ `git ls-remote --heads origin $BRANCH_NAME 2>/dev/null` ] + if [ -n `git ls-remote --heads origin $BRANCH_NAME 2>/dev/null` ] then echo "Staging deployment branch already exists!" else