From e979e4ec349c0fc00130ab39dcb9b431b852d5c3 Mon Sep 17 00:00:00 2001 From: shrouti1507 Date: Mon, 29 Jan 2024 14:14:49 +0530 Subject: [PATCH 1/6] fix: adding map for marketo known values and javascript known values --- .vscode/launch.json | 4 +- .../marketo_bulk_upload/config.js | 18 ++ .../marketo_bulk_upload.util.test.js | 185 ++++++++++++++++++ .../destinations/marketo_bulk_upload/util.js | 3 +- 4 files changed, 207 insertions(+), 3 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index d6feaf356e..bfd138bba1 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -19,7 +19,7 @@ "name": "Launch TS", "program": "${workspaceFolder}/dist/src/index.js", "outFiles": ["${workspaceFolder}/dist/src/**/*.js"], - "runtimeExecutable": "/usr/local/bin/node" + "runtimeExecutable": "/Users/shrouti/.nvm/versions/node/v18.19.0/bin/node" }, { "name": "benchmark", @@ -29,7 +29,7 @@ "args": ["--destinations=${input:destinations}", "--feature=${input:feature}"], "runtimeArgs": ["--nolazy"], "sourceMaps": true, - "runtimeExecutable": "/usr/local/bin/node" + "runtimeExecutable": "/Users/shrouti/.nvm/versions/node/v18.19.0/bin/node" }, { "runtimeExecutable": "/usr/local/bin/node", diff --git a/src/v0/destinations/marketo_bulk_upload/config.js b/src/v0/destinations/marketo_bulk_upload/config.js index 487e11fe24..36b3c2ff93 100644 --- a/src/v0/destinations/marketo_bulk_upload/config.js +++ b/src/v0/destinations/marketo_bulk_upload/config.js @@ -18,6 +18,23 @@ const FETCH_FAILURE_JOB_STATUS_ERR_MSG = 'Could not fetch failure job status'; const FETCH_WARNING_JOB_STATUS_ERR_MSG = 'Could not fetch warning job status'; const ACCESS_TOKEN_FETCH_ERR_MSG = 'Error during fetching access token'; +const SCHEMA_DATA_TYPE_MAP = { + string: 'string', + number: 'number', + boolean: 'boolean', + undefined: 'undefined', + float: 'number', + text: 'string', + currency: 'string', + integer: 'number', + reference: 'string', + datetime: 'string', + date: 'string', + email: 'string', + phone: 'string', + url: 'string', +}; + module.exports = { ABORTABLE_CODES, RETRYABLE_CODES, @@ -33,4 +50,5 @@ module.exports = { FETCH_FAILURE_JOB_STATUS_ERR_MSG, FETCH_WARNING_JOB_STATUS_ERR_MSG, ACCESS_TOKEN_FETCH_ERR_MSG, + SCHEMA_DATA_TYPE_MAP, }; diff --git a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js index 769fa4006d..922183fb9e 100644 --- a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js +++ b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js @@ -3,6 +3,7 @@ const { handlePollResponse, handleFileUploadResponse, getAccessToken, + checkEventStatusViaSchemaMatching, } = require('./util'); const { @@ -353,3 +354,187 @@ describe('getAccessToken', () => { await expect(getAccessToken(config)).rejects.toThrow(TransformationError); }); }); + +describe('checkEventStatusViaSchemaMatching', () => { + // The function correctly identifies fields with expected data types. + it('should correctly identify fields with expected data types', () => { + const event = { + input: [ + { + message: { + email: 'value1', + id: 123, + isLead: true, + }, + metadata: { + job_id: 'job1', + }, + }, + ], + }; + const fieldSchemaMapping = { + email: 'string', + id: 'integer', + isLead: 'boolean', + }; + + const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); + + expect(result).toEqual({}); + }); + + // The function correctly identifies fields with unexpected data types. + it('should correctly identify fields with unexpected data types', () => { + const event = { + input: [ + { + message: { + email: 123, + city: '123', + islead: true, + }, + metadata: { + job_id: 'job1', + }, + }, + ], + }; + const fieldSchemaMapping = { + email: 'string', + city: 'number', + islead: 'boolean', + }; + + const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); + + expect(result).toEqual({ + job1: 'invalid email', + }); + }); + + // The function correctly handles events with multiple fields. + it('should correctly handle events with multiple fields', () => { + const event = { + input: [ + { + message: { + id: 'value1', + testCustomFieldScore: 123, + isLead: true, + }, + metadata: { + job_id: 'job1', + }, + }, + { + message: { + email: 'value2', + id: 456, + testCustomFieldScore: false, + }, + metadata: { + job_id: 'job2', + }, + }, + ], + }; + const fieldSchemaMapping = { + email: 'string', + id: 'integer', + testCustomFieldScore: 'integer', + isLead: 'boolean', + }; + + const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); + + expect(result).toEqual({ + job1: 'invalid id', + job2: 'invalid testCustomFieldScore', + }); + }); + + // The function correctly handles events with missing fields. + it('should have no effect with missing fields', () => { + const event = { + input: [ + { + message: { + field1: 'value1', + field3: true, + }, + metadata: { + job_id: 'job1', + }, + }, + ], + }; + const fieldSchemaMapping = { + field1: 'string', + field2: 'number', + field3: 'boolean', + }; + + const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); + + expect(result).toEqual({}); + }); + + // The function correctly handles events with additional fields. But this will not happen in our use case + it('should correctly handle events with additional fields', () => { + const event = { + input: [ + { + message: { + email: 'value1', + id: 124, + isLead: true, + abc: 'value2', + }, + metadata: { + job_id: 'job1', + }, + }, + ], + }; + const fieldSchemaMapping = { + email: 'string', + id: 'number', + isLead: 'boolean', + }; + + const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); + + expect(result).toEqual({ + job1: 'invalid abc', + }); + }); + + // The function correctly handles events with null values. + it('should correctly handle events with null values', () => { + const event = { + input: [ + { + message: { + email: 'value1', + id: null, + isLead: true, + }, + metadata: { + job_id: 'job1', + }, + }, + ], + }; + const fieldSchemaMapping = { + email: 'string', + id: 'number', + isLead: 'boolean', + }; + + const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); + + expect(result).toEqual({ + job1: 'invalid id', + }); + }); +}); diff --git a/src/v0/destinations/marketo_bulk_upload/util.js b/src/v0/destinations/marketo_bulk_upload/util.js index 8b46212b87..fb6a0b1365 100644 --- a/src/v0/destinations/marketo_bulk_upload/util.js +++ b/src/v0/destinations/marketo_bulk_upload/util.js @@ -18,6 +18,7 @@ const { POLL_STATUS_ERR_MSG, FILE_UPLOAD_ERR_MSG, ACCESS_TOKEN_FETCH_ERR_MSG, + SCHEMA_DATA_TYPE_MAP, } = require('./config'); const logger = require('../../../logger'); @@ -401,7 +402,7 @@ const checkEventStatusViaSchemaMatching = (event, fieldMap) => { const { job_id } = metadata; Object.entries(message).forEach(([paramName, paramValue]) => { - let expectedDataType = fieldMap[paramName]; + let expectedDataType = SCHEMA_DATA_TYPE_MAP[fieldMap[paramName]]; const actualDataType = typeof paramValue; // If expectedDataType is not one of the primitive data types, treat it as a string From 8763caaddbdef7c8dd014e309db04479c43e345c Mon Sep 17 00:00:00 2001 From: shrouti1507 Date: Mon, 29 Jan 2024 14:20:22 +0530 Subject: [PATCH 2/6] fix: reverting launch.json to original state --- .vscode/launch.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index bfd138bba1..d6feaf356e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -19,7 +19,7 @@ "name": "Launch TS", "program": "${workspaceFolder}/dist/src/index.js", "outFiles": ["${workspaceFolder}/dist/src/**/*.js"], - "runtimeExecutable": "/Users/shrouti/.nvm/versions/node/v18.19.0/bin/node" + "runtimeExecutable": "/usr/local/bin/node" }, { "name": "benchmark", @@ -29,7 +29,7 @@ "args": ["--destinations=${input:destinations}", "--feature=${input:feature}"], "runtimeArgs": ["--nolazy"], "sourceMaps": true, - "runtimeExecutable": "/Users/shrouti/.nvm/versions/node/v18.19.0/bin/node" + "runtimeExecutable": "/usr/local/bin/node" }, { "runtimeExecutable": "/usr/local/bin/node", From 76b2cdfb5d945f36b621d8206d3116dffe5b189a Mon Sep 17 00:00:00 2001 From: shrouti1507 Date: Mon, 29 Jan 2024 16:05:29 +0530 Subject: [PATCH 3/6] fix: editing test cases --- .../marketo_bulk_upload.util.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js index 922183fb9e..8d40d303ed 100644 --- a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js +++ b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js @@ -439,7 +439,7 @@ describe('checkEventStatusViaSchemaMatching', () => { ], }; const fieldSchemaMapping = { - email: 'string', + email: 'email', id: 'integer', testCustomFieldScore: 'integer', isLead: 'boolean', @@ -459,8 +459,8 @@ describe('checkEventStatusViaSchemaMatching', () => { input: [ { message: { - field1: 'value1', - field3: true, + email: 'value1', + isLead: true, }, metadata: { job_id: 'job1', @@ -469,9 +469,9 @@ describe('checkEventStatusViaSchemaMatching', () => { ], }; const fieldSchemaMapping = { - field1: 'string', - field2: 'number', - field3: 'boolean', + email: 'string', + id: 'number', + isLead: 'boolean', }; const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); From 952fda6c33dc3918f6108d8695b388b76fa2d3f8 Mon Sep 17 00:00:00 2001 From: shrouti1507 Date: Mon, 29 Jan 2024 21:15:05 +0530 Subject: [PATCH 4/6] fix: review comments addressed --- src/v0/destinations/marketo_bulk_upload/config.js | 1 + .../marketo_bulk_upload.util.test.js | 10 +++++----- src/v0/destinations/marketo_bulk_upload/util.js | 7 +------ 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/v0/destinations/marketo_bulk_upload/config.js b/src/v0/destinations/marketo_bulk_upload/config.js index 36b3c2ff93..e3268711fe 100644 --- a/src/v0/destinations/marketo_bulk_upload/config.js +++ b/src/v0/destinations/marketo_bulk_upload/config.js @@ -33,6 +33,7 @@ const SCHEMA_DATA_TYPE_MAP = { email: 'string', phone: 'string', url: 'string', + object: 'object', }; module.exports = { diff --git a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js index 8d40d303ed..875b0d8280 100644 --- a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js +++ b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js @@ -357,7 +357,7 @@ describe('getAccessToken', () => { describe('checkEventStatusViaSchemaMatching', () => { // The function correctly identifies fields with expected data types. - it('should correctly identify fields with expected data types', () => { + it('if event data types match with expected data types we send no field as mismatch', () => { const event = { input: [ { @@ -384,7 +384,7 @@ describe('checkEventStatusViaSchemaMatching', () => { }); // The function correctly identifies fields with unexpected data types. - it('should correctly identify fields with unexpected data types', () => { + it('if event data types do not match with expected data types we send that field as mismatch', () => { const event = { input: [ { @@ -413,7 +413,7 @@ describe('checkEventStatusViaSchemaMatching', () => { }); // The function correctly handles events with multiple fields. - it('should correctly handle events with multiple fields', () => { + it('For array of events the mismatch object fills up with each event errors', () => { const event = { input: [ { @@ -454,7 +454,7 @@ describe('checkEventStatusViaSchemaMatching', () => { }); // The function correctly handles events with missing fields. - it('should have no effect with missing fields', () => { + it('it is not mandatory to send all the fields present in schema', () => { const event = { input: [ { @@ -480,7 +480,7 @@ describe('checkEventStatusViaSchemaMatching', () => { }); // The function correctly handles events with additional fields. But this will not happen in our use case - it('should correctly handle events with additional fields', () => { + it('for any field beyond schema fields will be mapped as invalid', () => { const event = { input: [ { diff --git a/src/v0/destinations/marketo_bulk_upload/util.js b/src/v0/destinations/marketo_bulk_upload/util.js index fb6a0b1365..fac04af431 100644 --- a/src/v0/destinations/marketo_bulk_upload/util.js +++ b/src/v0/destinations/marketo_bulk_upload/util.js @@ -402,14 +402,9 @@ const checkEventStatusViaSchemaMatching = (event, fieldMap) => { const { job_id } = metadata; Object.entries(message).forEach(([paramName, paramValue]) => { - let expectedDataType = SCHEMA_DATA_TYPE_MAP[fieldMap[paramName]]; + const expectedDataType = SCHEMA_DATA_TYPE_MAP[fieldMap[paramName]]; const actualDataType = typeof paramValue; - // If expectedDataType is not one of the primitive data types, treat it as a string - if (!['string', 'number', 'boolean', 'undefined'].includes(expectedDataType)) { - expectedDataType = 'string'; - } - if (!mismatchedFields[job_id] && actualDataType !== expectedDataType) { mismatchedFields[job_id] = `invalid ${paramName}`; } From d7d6bfbbb85060ee75d34746296838075e0c8866 Mon Sep 17 00:00:00 2001 From: shrouti1507 Date: Tue, 30 Jan 2024 17:25:54 +0530 Subject: [PATCH 5/6] fix: making error statement better --- .../marketo_bulk_upload.util.test.js | 10 +++++----- src/v0/destinations/marketo_bulk_upload/util.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js index 875b0d8280..c41fa6855b 100644 --- a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js +++ b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js @@ -408,7 +408,7 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid email', + job1: 'invalid email for value 123 ', }); }); @@ -448,8 +448,8 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid id', - job2: 'invalid testCustomFieldScore', + job1: 'invalid id for value value1 ', + job2: 'invalid testCustomFieldScore for value false ', }); }); @@ -505,7 +505,7 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid abc', + job1: 'invalid abc for value value2 ', }); }); @@ -534,7 +534,7 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid id', + job1: 'invalid id for value null ', }); }); }); diff --git a/src/v0/destinations/marketo_bulk_upload/util.js b/src/v0/destinations/marketo_bulk_upload/util.js index fac04af431..70f8509bba 100644 --- a/src/v0/destinations/marketo_bulk_upload/util.js +++ b/src/v0/destinations/marketo_bulk_upload/util.js @@ -406,7 +406,7 @@ const checkEventStatusViaSchemaMatching = (event, fieldMap) => { const actualDataType = typeof paramValue; if (!mismatchedFields[job_id] && actualDataType !== expectedDataType) { - mismatchedFields[job_id] = `invalid ${paramName}`; + mismatchedFields[job_id] = `invalid ${paramName} for value ${paramValue} `; } }); }); From f7bf0cf05bcabd90003483f59475b9a278d05df4 Mon Sep 17 00:00:00 2001 From: shrouti1507 Date: Wed, 31 Jan 2024 10:06:30 +0530 Subject: [PATCH 6/6] feat: reviw comments addressed --- .../marketo_bulk_upload.util.test.js | 10 +++++----- src/v0/destinations/marketo_bulk_upload/util.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js index c41fa6855b..875b0d8280 100644 --- a/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js +++ b/src/v0/destinations/marketo_bulk_upload/marketo_bulk_upload.util.test.js @@ -408,7 +408,7 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid email for value 123 ', + job1: 'invalid email', }); }); @@ -448,8 +448,8 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid id for value value1 ', - job2: 'invalid testCustomFieldScore for value false ', + job1: 'invalid id', + job2: 'invalid testCustomFieldScore', }); }); @@ -505,7 +505,7 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid abc for value value2 ', + job1: 'invalid abc', }); }); @@ -534,7 +534,7 @@ describe('checkEventStatusViaSchemaMatching', () => { const result = checkEventStatusViaSchemaMatching(event, fieldSchemaMapping); expect(result).toEqual({ - job1: 'invalid id for value null ', + job1: 'invalid id', }); }); }); diff --git a/src/v0/destinations/marketo_bulk_upload/util.js b/src/v0/destinations/marketo_bulk_upload/util.js index 70f8509bba..fac04af431 100644 --- a/src/v0/destinations/marketo_bulk_upload/util.js +++ b/src/v0/destinations/marketo_bulk_upload/util.js @@ -406,7 +406,7 @@ const checkEventStatusViaSchemaMatching = (event, fieldMap) => { const actualDataType = typeof paramValue; if (!mismatchedFields[job_id] && actualDataType !== expectedDataType) { - mismatchedFields[job_id] = `invalid ${paramName} for value ${paramValue} `; + mismatchedFields[job_id] = `invalid ${paramName}`; } }); });