fix: refactor code and add validation for values (#3971)

* fix: refactor code and add validation for values * chore: update updateConversion conditions to mutually exclusive
rudderlabs · Jan 20, 2025 · fc09080 · fc09080
1 parent 6ec3d55
commit fc09080
Show file tree

Hide file tree

Showing 6 changed files with 163 additions and 25 deletions.
diff --git a/.../destinations/google_adwords_offline_conversions/data/TrackAddStoreConversionsConfig.json b/.../destinations/google_adwords_offline_conversions/data/TrackAddStoreConversionsConfig.json
@@ -17,7 +17,8 @@
     ],
     "required": true,
     "metadata": {
-      "type": "toNumber"
+      "type": "toNumber",
+      "regex": "^([1-9]\\d*(\\.\\d+)?|0\\.\\d+)$"
     }
   },
   {

diff --git a/src/v0/destinations/google_adwords_offline_conversions/transform.js b/src/v0/destinations/google_adwords_offline_conversions/transform.js
@@ -18,7 +18,6 @@ const {
   getClickConversionPayloadAndEndpoint,
   getConsentsDataFromIntegrationObj,
   getCallConversionPayload,
-  updateConversion,
 } = require('./utils');
 const helper = require('./helper');
 
@@ -49,9 +48,6 @@ const getConversions = (message, metadata, { Config }, event, conversionType) =>
       filteredCustomerId,
       eventLevelConsentsData,
     );
-    convertedPayload.payload.conversions[0] = updateConversion(
-      convertedPayload.payload.conversions[0],
-    );
     payload = convertedPayload.payload;
     endpoint = convertedPayload.endpoint;
   } else if (conversionType === 'store') {

diff --git a/src/v0/destinations/google_adwords_offline_conversions/utils.js b/src/v0/destinations/google_adwords_offline_conversions/utils.js
@@ -346,6 +346,24 @@ const populateUserIdentifier = ({ email, phone, properties, payload, UserIdentif
   }
   return copiedPayload;
 };
+
+/**
+ * remove redundant ids
+ * @param {*} conversionCopy
+ */
+const updateConversion = (conversion) => {
+  const conversionCopy = cloneDeep(conversion);
+  if (conversionCopy.gclid) {
+    delete conversionCopy.wbraid;
+    delete conversionCopy.gbraid;
+  } else if (conversionCopy.wbraid && conversionCopy.gbraid) {
+    throw new InstrumentationError(`You can't use both wbraid and gbraid.`);
+  } else if (conversionCopy.wbraid || conversionCopy.gbraid) {
+    delete conversionCopy.userIdentifiers;
+  }
+  return conversionCopy;
+};
+
 const getClickConversionPayloadAndEndpoint = (
   message,
   Config,
@@ -423,6 +441,7 @@ const getClickConversionPayloadAndEndpoint = (
   const consentObject = finaliseConsent(consentConfigMap, eventLevelConsent, Config);
   // here conversions[0] is expected to be present there are some mandatory properties mapped in the mapping json.
   set(payload, 'conversions[0].consent', consentObject);
+  payload.conversions[0] = updateConversion(payload.conversions[0]);
   return { payload, endpoint };
 };
 
@@ -431,25 +450,6 @@ const getConsentsDataFromIntegrationObj = (message) => {
   return integrationObj?.consents || {};
 };
 
-/**
- * remove redundant ids
- * @param {*} conversionCopy
- */
-const updateConversion = (conversion) => {
-  const conversionCopy = cloneDeep(conversion);
-  if (conversionCopy.gclid) {
-    delete conversionCopy.wbraid;
-    delete conversionCopy.gbraid;
-  }
-  if (conversionCopy.wbraid && conversionCopy.gbraid) {
-    throw new InstrumentationError(`You can't use both wbraid and gbraid.`);
-  }
-  if (conversionCopy.wbraid || conversionCopy.gbraid) {
-    delete conversionCopy.userIdentifiers;
-  }
-  return conversionCopy;
-};
-
 module.exports = {
   validateDestinationConfig,
   generateItemListFromProducts,

diff --git a/src/v0/util/index.js b/src/v0/util/index.js
@@ -971,6 +971,7 @@ const handleMetadataForValue = (value, metadata, destKey, integrationsObj = null
     validateTimestamp,
     allowedKeyCheck,
     toArray,
+    regex,
   } = metadata;
 
   // if value is null and defaultValue is supplied - use that
@@ -1044,7 +1045,14 @@ const handleMetadataForValue = (value, metadata, destKey, integrationsObj = null
     }
     return [formattedVal];
   }
-
+  if (regex) {
+    const regexPattern = new RegExp(regex);
+    if (!regexPattern.test(formattedVal)) {
+      throw new InstrumentationError(
+        `The value '${formattedVal}' does not match the regex pattern, ${regex}`,
+      );
+    }
+  }
   return formattedVal;
 };
 
@@ -2489,4 +2497,5 @@ module.exports = {
   removeEmptyKey,
   isAxiosError,
   convertToUuid,
+  handleMetadataForValue,
 };
diff --git a/src/v0/util/index.test.js b/src/v0/util/index.test.js
@@ -1049,3 +1049,27 @@ describe('convertToUuid', () => {
     expect(result).toBe('672ca00c-37f4-5d71-b8c3-6ae0848080ec');
   });
 });
+
+describe('', () => {
+  it('should return original value when regex pattern is invalid', () => {
+    const value = 'test value';
+    const metadata = {
+      regex: `\\b(?!1000\\b)\\d{4,}\\b`,
+    };
+    try {
+      const result = utilities.handleMetadataForValue(value, metadata);
+    } catch (e) {
+      expect(e.message).toBe(
+        `The value 'test value' does not match the regex pattern, \\b(?!1000\\b)\\d{4,}\\b`,
+      );
+    }
+  });
+  it('should return true when the regex matches', () => {
+    const value = 1003;
+    const metadata = {
+      regex: `\\b(?!1000\\b)\\d{4,}\\b`,
+    };
+    const res = utilities.handleMetadataForValue(value, metadata);
+    expect(res).toBe(1003);
+  });
+});
diff --git a/test/integrations/destinations/google_adwords_offline_conversions/processor/data.ts b/test/integrations/destinations/google_adwords_offline_conversions/processor/data.ts
@@ -5929,4 +5929,112 @@ export const data = [
     },
     mockFns: timestampMock,
   },
+  {
+    name: 'google_adwords_offline_conversions',
+    description: 'Test 29 : store conversion which has value less than 0, should throw error',
+    feature: 'processor',
+    module: 'destination',
+    version: 'v0',
+    input: {
+      request: {
+        body: [
+          {
+            message: {
+              channel: 'web',
+              context: {
+                traits: {},
+              },
+              event: 'Product Clicked',
+              type: 'track',
+              messageId: '5e10d13a-bf9a-44bf-b884-43a9e591ea71',
+              anonymousId: '00000000000000000000000000',
+              userId: '12345',
+              properties: {
+                item_id: 'item id',
+                merchant_id: 'merchant id',
+                currency: 'INR',
+                revenue: '0',
+                store_code: 'store code',
+                gclid: 'gclid',
+                conversionDateTime: '2019-10-14T11:15:18.299Z',
+                product_id: '123445',
+                quantity: 123,
+              },
+              integrations: {
+                google_adwords_offline_conversion: {
+                  consent: {
+                    adUserdata: 'UNSPECIFIED',
+                    adPersonalization: 'GRANTED',
+                  },
+                },
+              },
+              name: 'ApplicationLoaded',
+              sentAt: '2019-10-14T11:15:53.296Z',
+            },
+            metadata: {
+              secret: {
+                access_token: 'abcd1234',
+                refresh_token: 'efgh5678',
+                developer_token: 'ijkl91011',
+              },
+            },
+            destination: {
+              Config: {
+                isCustomerAllowed: false,
+                customerId: '111-222-3333',
+                subAccount: true,
+                loginCustomerId: 'login-customer-id',
+                userDataConsent: 'GRANTED',
+                personalizationConsent: 'DENIED',
+                eventsToOfflineConversionsTypeMapping: [
+                  {
+                    from: 'Product Clicked',
+                    to: 'store',
+                  },
+                ],
+                eventsToConversionsNamesMapping: [
+                  {
+                    from: 'Product Clicked',
+                    to: 'Sign-up - click',
+                  },
+                ],
+                hashUserIdentifier: true,
+                defaultUserIdentifier: 'phone',
+                validateOnly: false,
+                rudderAccountId: '2EOknn1JNH7WK1MfNkgr4t3u4fGYKkRK',
+              },
+            },
+          },
+        ],
+      },
+    },
+    output: {
+      response: {
+        status: 200,
+        body: [
+          {
+            error:
+              "The value '0' does not match the regex pattern, ^([1-9]\\d*(\\.\\d+)?|0\\.\\d+)$",
+            metadata: {
+              secret: {
+                access_token: 'abcd1234',
+                developer_token: 'ijkl91011',
+                refresh_token: 'efgh5678',
+              },
+            },
+            statTags: {
+              destType: 'GOOGLE_ADWORDS_OFFLINE_CONVERSIONS',
+              errorCategory: 'dataValidation',
+              errorType: 'instrumentation',
+              feature: 'processor',
+              implementation: 'native',
+              module: 'destination',
+            },
+            statusCode: 400,
+          },
+        ],
+      },
+    },
+    mockFns: timestampMock,
+  },
 ];