fix: update proxy data type for response handler input #3030
Conversation
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe updates across the system primarily involve renaming response and request types to reflect versioning, with Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #3030 +/- ##
===========================================
- Coverage 87.23% 84.51% -2.73%
===========================================
Files 541 493 -48
Lines 29325 27301 -2024
Branches 6995 6569 -426
===========================================
- Hits 25583 23073 -2510
- Misses 3395 3800 +405
- Partials 347 428 +81 ☔ View full report in Codecov by Sentry. |
utsabc
changed the title
|
@coderabbitai review |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (33)
- src/adapters/networkhandler/genericNetworkHandler.js (1 hunks)
- src/controllers/delivery.ts (5 hunks)
- src/interfaces/DestinationService.ts (3 hunks)
- src/services/comparator.ts (2 hunks)
- src/services/destination/cdkV1Integration.ts (3 hunks)
- src/services/destination/cdkV2Integration.ts (3 hunks)
- src/services/destination/nativeIntegration.ts (6 hunks)
- src/services/destination/postTransformation.ts (5 hunks)
- src/types/index.ts (8 hunks)
- src/v0/destinations/adobe_analytics/networkHandler.js (2 hunks)
- src/v0/destinations/braze/networkHandler.js (1 hunks)
- src/v0/destinations/campaign_manager/networkHandler.js (1 hunks)
- src/v0/destinations/clevertap/networkHandler.js (1 hunks)
- src/v0/destinations/criteo_audience/networkHandler.js (1 hunks)
- src/v0/destinations/fb/networkHandler.js (1 hunks)
- src/v0/destinations/ga4/networkHandler.js (3 hunks)
- src/v0/destinations/google_adwords_enhanced_conversions/networkHandler.js (1 hunks)
- src/v0/destinations/google_adwords_offline_conversions/networkHandler.js (1 hunks)
- src/v0/destinations/google_adwords_remarketing_lists/networkHandler.js (1 hunks)
- src/v0/destinations/intercom/networkHandler.js (1 hunks)
- src/v0/destinations/marketo/networkHandler.js (1 hunks)
- src/v0/destinations/marketo_static_list/networkHandler.js (1 hunks)
- src/v0/destinations/pardot/networkHandler.js (1 hunks)
- src/v0/destinations/reddit/networkHandler.js (1 hunks)
- src/v0/destinations/salesforce/networkHandler.js (1 hunks)
- src/v0/destinations/salesforce_oauth/networkHandler.js (1 hunks)
- src/v0/destinations/snapchat_custom_audience/networkHandler.js (1 hunks)
- src/v0/destinations/the_trade_desk/networkHandler.js (1 hunks)
- src/v0/destinations/tiktok_ads/networkHandler.js (1 hunks)
- src/v0/util/facebookUtils/networkHandler.js (1 hunks)
- src/v1/destinations/campaign_manager/networkHandler.js (1 hunks)
- test/integrations/destinations/campaign_manager/dataDelivery/data.ts (3 hunks)
- test/integrations/destinations/salesforce/dataDelivery/data.ts (10 hunks)
Additional comments: 49
src/v0/destinations/fb/networkHandler.js (1)
- 5-6: The change to use
responseParamsas an argument and then destructuredestinationResponsefrom it is a good practice for future extensibility. This allows for additional parameters to be passed without changing the function signature.src/v0/destinations/salesforce/networkHandler.js (2)
- 6-7: The refactoring to accept
responseParamsand then extractdestinationResponse,destType, andrudderJobMetadatais consistent with the changes in other network handlers and is a good practice for extensibility.- 13-13: The optional chaining on
rudderJobMetadata?.destInfo?.authKeyis a safe way to access nested properties which may not always be present.src/v0/destinations/salesforce_oauth/networkHandler.js (2)
- 6-7: The changes are consistent with the previous file, using
responseParamsto pass in the function arguments. This maintains consistency across the codebase.- 13-13: The use of optional chaining here is appropriate and ensures that the code does not throw an error if
rudderJobMetadataor nested properties are undefined.src/v0/destinations/marketo/networkHandler.js (2)
- 6-7: Refactoring to use
responseParamsand destructuring within the function is a good approach for handling potential future changes to the input parameters.- 9-9: The destructuring of
statusdirectly fromdestinationResponseis clean and readable.src/v0/destinations/intercom/networkHandler.js (1)
- 16-17: The change to use
responseParamsand then destructuredestinationResponseanddestTypeis consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/marketo_static_list/networkHandler.js (2)
- 7-8: The refactoring to accept
responseParamsand then extract the necessary variables is consistent with the changes in other network handlers and is a good practice for extensibility.- 10-10: The separate line for destructuring
statusfromdestinationResponseis clean and improves readability.src/interfaces/DestinationService.ts (2)
- 1-5: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [2-11]
The renaming of types from
DeliveryResponsetoDeliveryV0ResponseandDeliveriesResponsetoDeliveryV1Responseis consistent with the PR objectives to align type definitions across the system.
- 52-52: The updated return type for the
delivermethod to eitherDeliveryV0ResponseorDeliveryV1Responsereflects the changes in the proxy API versioning and is in line with the PR objectives.src/v0/destinations/tiktok_ads/networkHandler.js (1)
- 11-12: The change to use
responseParamsand then destructuredestinationResponsefrom it is consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/clevertap/networkHandler.js (1)
- 10-11: The refactoring to accept
responseParamsand then extractdestinationResponseis consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/adobe_analytics/networkHandler.js (2)
- 18-19: The change to use
responseParamsand then destructuredestinationResponseanddestTypeis consistent with the changes in other network handlers and is a good practice for extensibility.- 32-36: The use of
destTypeinstead ofdestin the error messages is consistent with the destructured variable name and avoids confusion.src/v0/destinations/reddit/networkHandler.js (1)
- 21-22: The change to use
responseParamsand then destructuredestinationResponsefrom it is consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/braze/networkHandler.js (1)
- 14-15: The refactoring to accept
responseParamsand then extractdestinationResponseis consistent with the changes in other network handlers and is a good practice for extensibility.src/adapters/networkhandler/genericNetworkHandler.js (1)
- 20-21: The change to use
responseParamsand then destructuredestinationResponseanddestTypeis consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/ga4/networkHandler.js (2)
- 11-12: The change to use
responseParamsand then destructuredestinationResponseanddestTypeis consistent with the changes in other network handlers and is a good practice for extensibility.- 30-36: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [33-46]
The detailed error handling for GA4's specific error codes and messages is well-implemented and ensures that the correct type of error is thrown based on the response.
src/v0/destinations/campaign_manager/networkHandler.js (1)
- 47-48: The change to use
responseParamsand then destructuredestinationResponsefrom it is consistent with the changes in other network handlers and is a good practice for extensibility.src/v1/destinations/campaign_manager/networkHandler.js (1)
- 37-38: The change to use
responseParamsand then destructuredestinationResponseandrudderJobMetadatais consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/the_trade_desk/networkHandler.js (1)
- 44-45: The change to use
responseParamsand then destructuredestinationResponsefrom it is consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/snapchat_custom_audience/networkHandler.js (1)
- 83-84: The change to use
responseParamsand then destructuredestinationResponsefrom it is consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/pardot/networkHandler.js (1)
- 68-69: The change to use
responseParamsand then destructuredestinationResponsefrom it is consistent with the changes in other network handlers and is a good practice for extensibility.src/v0/destinations/criteo_audience/networkHandler.js (1)
- 70-71: The change to use
responseParamsand then destructuredestinationResponsefrom it is consistent with the changes in other network handlers and is a good practice for extensibility.src/services/destination/cdkV1Integration.ts (2)
- 4-10: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [7-17]
The renaming of types from
DeliveryResponsetoDeliveryV0ResponseandDeliveriesResponsetoDeliveryV1Responseis consistent with the PR objectives to align type definitions across the system.
- 124-124: The updated return type for the
delivermethod to eitherDeliveryV0ResponseorDeliveryV1Responsereflects the changes in the proxy API versioning and is in line with the PR objectives.src/controllers/delivery.ts (3)
- 6-10: The changes in the
DeliveryControllerclass to useDeliveryV0ResponseandProxyV0Requestfor thedeliverToDestinationmethod, andDeliveryV1ResponseandProxyV1Requestfor thedeliverToDestinationV1method, are consistent with the PR objectives and the updated type definitions.- 22-30: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-36]
The try-catch block in
deliverToDestinationmethod is correctly handling the delivery response and catching any errors that may occur during the delivery process.
- 57-65: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [60-71]
The try-catch block in
deliverToDestinationV1method is correctly handling the delivery response and catching any errors that may occur during the delivery process.src/v0/destinations/google_adwords_enhanced_conversions/networkHandler.js (1)
- 105-107: The extraction of
destinationResponsefromresponseParamsis correctly implemented.src/v0/destinations/google_adwords_remarketing_lists/networkHandler.js (2)
- 156-157: The change from a direct parameter to an object
responseParamsis consistent with the changes in other network handlers. Ensure that all calls toresponseHandlerhave been updated accordingly.- 156-157: The extraction of
destinationResponsefromresponseParamsis correctly implemented.src/services/destination/cdkV2Integration.ts (1)
- 173-173: The method signature change from
DeliveryResponse | DeliveriesResponsetoDeliveryV0Response | DeliveryV1Responsealigns with the updated type definitions.src/services/destination/postTransformation.ts (2)
- 11-14: The renaming of response types from
DeliveryResponsetoDeliveryV0Responseand the addition ofDeliveryV1Responseis consistent with the changes in the type definitions.- 164-170: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [167-192]
The addition of the
handlevV1DeliveriesFailureEventsmethod is a good extension to handle v1 delivery failure events. Ensure that this method is being used correctly in the delivery process.src/types/index.ts (3)
- 18-24: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [21-36]
The renaming of
ProxyDeliveryRequesttoProxyV0Requestand the change of themetadatafield toProxyMetdatais consistent with the updated type definitions.
- 51-74: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [39-55]
The renaming of
ProxyDeliveriesRequesttoProxyV1Requestand the addition of new fields such asdestinationConfiganddontBatchare good for aligning with the updated proxy API versioning.
- 197-208: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [189-205]
The renaming of
DeliveryResponseandDeliveriesResponsetoDeliveryV0ResponseandDeliveryV1Responserespectively, and the introduction of the new typeProxyMetdataare consistent with the PR objectives to align type definitions.src/v0/util/facebookUtils/networkHandler.js (1)
- 252-253: The modification to accept
responseParamsinstead of a directdestinationResponseparameter is consistent with the changes in other network handlers.src/v0/destinations/google_adwords_offline_conversions/networkHandler.js (2)
- 254-255: The change from a direct parameter to an object
responseParamsis consistent with the changes in other network handlers. Ensure that all calls toresponseHandlerhave been updated accordingly.- 254-255: The extraction of
destinationResponsefromresponseParamsis correctly implemented.src/services/destination/nativeIntegration.ts (2)
- 16-24: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [8-21]
The renaming of types and updating of method signatures to return either
DeliveryV0ResponseorDeliveryV1Responseis consistent with the updated type definitions.
- 191-212: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [184-222]
The logic to handle the response based on the version and handler is correctly implemented. However, ensure that the adaption logic for V0 to V1 is tested thoroughly to prevent any data inconsistencies.
src/services/comparator.ts (1)
- 373-373: The method signature for
deliverhas been updated to return a union type ofDeliveryV0Response | DeliveryV1Response. Ensure that all usages of this method are updated to handle the new return types correctly.test/integrations/destinations/campaign_manager/dataDelivery/data.ts (1)
- 505-519: The metadata structure within the test data has been changed from an object to an array of objects. Verify that all test cases and related code correctly handle this new structure.
test/integrations/destinations/salesforce/dataDelivery/data.ts (1)
- 58-58: It appears that there are multiple deletions of
statTagsobjects in the test data. Confirm that these deletions are intentional and that no functionality is dependent on these objects.
| const responseHandler = (responseParams) => { | ||
| const { destinationResponse } = responseParams; | ||
| const message = 'Request Processed Successfully'; |
There was a problem hiding this comment.
Tip
Codebase Verification
The output from the rg command with context for legacy/router.js shows that responseHandler is being called with an object that includes ...processedProxyResponse and rudderJobMetadata: metadata, which seems to align with the updated signature that expects an object. However, there is an additional parameter destination being passed, which is not part of the responseParams object structure as per the snippet provided in the review comment.
Given this information, it appears that not all calls to responseHandler have been updated to match the new signature that expects a single responseParams object. Specifically, the calls in the marketo_bulk_upload files are passing arguments directly rather than as part of an object, and the call in legacy/router.js includes an additional parameter that is not encapsulated within an object.
Based on the evidence from the codebase, the review comment is correct in stating that all calls to responseHandler should be checked and updated accordingly to match the new signature.
Analysis chain
The change from a direct parameter to an object responseParams is good for future extensibility, but ensure that all calls to responseHandler have been updated accordingly.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if responseHandler is called with the new responseParams object structure.
ast-grep --lang javascript --pattern $'responseHandler($$$)' src/v0/destinations/google_adwords_enhanced_conversions/networkHandler.js
Length of output: 135
Script:
#!/bin/bash
# Search the entire codebase for calls to `responseHandler` to verify if it's called with the new `responseParams` object structure.
rg 'responseHandler\('
Length of output: 525
Script:
#!/bin/bash
# Verify the call to `responseHandler` in `legacy/router.js` to ensure it matches the new signature.
rg 'responseHandler\(' src/legacy/router.js
Length of output: 96
Script:
#!/bin/bash
# Extract more context around the call to `responseHandler` in `legacy/router.js` to verify the signature.
rg -C 3 'responseHandler\(' src/legacy/router.js
Length of output: 278
| }, | ||
| destinationType, | ||
| ); | ||
| const responseParams = { |
There was a problem hiding this comment.
This is the core part where the change has been done which affects all network handlers
Added new structure for proxy test scnearios for cm360 also added zod validations as part of tests
…usiness.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…usiness.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…to fix.network-handlers
…ransformer into fix.network-handlers
This reverts commit 689b0cd.
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
1 similar comment
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
| @@ -3,9 +3,10 @@ const { marketoResponseHandler } = require('./util'); | |||
| const { proxyRequest, prepareProxyRequest } = require('../../../adapters/network'); | |||
| const { processAxiosResponse } = require('../../../adapters/utils/networkUtils'); | |||
|
|
|||
| const responseHandler = (destinationResponse, destType) => { | |||
| const responseHandler = (responseParams) => { | |||
| const { destinationResponse, destType,rudderJobMetadata } = responseParams; | |||
There was a problem hiding this comment.
| const { destinationResponse, destType,rudderJobMetadata } = responseParams; | |
| const { destinationResponse, destType, rudderJobMetadata } = responseParams; |
| @@ -2,7 +2,8 @@ const { processAxiosResponse } = require('../../../adapters/utils/networkUtils') | |||
| const { errorResponseHandler } = require('../facebook_pixel/networkHandler'); | |||
| const { prepareProxyRequest, proxyRequest } = require('../../../adapters/network'); | |||
|
|
|||
| const destResponseHandler = (destinationResponse) => { | |||
| const destResponseHandler = (responseParams) => { | |||
There was a problem hiding this comment.
Lets rename destResponseHandler to responseHandler?
| @@ -13,8 +13,9 @@ const errorResponseHandler = (destinationResponse, dest) => { | |||
| } | |||
| }; | |||
|
|
|||
| const destResponseHandler = (destinationResponse, dest) => { | |||
| errorResponseHandler(destinationResponse, dest); | |||
| const destResponseHandler = (responseParams) => { | |||
There was a problem hiding this comment.
Lets rename destResponseHandler to responseHandler?
| @@ -275,7 +275,8 @@ const errorResponseHandler = (destResponse) => { | |||
| ); | |||
| }; | |||
|
|
|||
| const destResponseHandler = (destinationResponse) => { | |||
| const destResponseHandler = (responseParams) => { | |||
There was a problem hiding this comment.
Lets rename destResponseHandler to responseHandler?
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
* chore: update delivery test cases for criteo audience
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
* chore: refactor braze proxy v1 tests * chore: address review comments and cleanup * chore: cleanup of mock --------- Co-authored-by: Utsab Chowdhury <utsabc@Utsabs-MacBook-Pro.local>
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/3030/test-report.html |
|
@coderabbitai review |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 14
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
package-lock.jsonis excluded by:!**/*.jsonpackage.jsonis excluded by:!**/*.json
Files selected for processing (42)
- src/controllers/delivery.ts (6 hunks)
- src/services/destination/postTransformation.ts (6 hunks)
- src/types/index.ts (11 hunks)
- src/types/zodTypes.ts (1 hunks)
- src/v0/destinations/am/util.test.js (2 hunks)
- src/v0/destinations/am/utils.js (2 hunks)
- src/v0/destinations/ga4/networkHandler.js (3 hunks)
- src/v0/destinations/rakuten/networkHandler.js (2 hunks)
- src/v0/destinations/rakuten/networkHandler.test.js (5 hunks)
- src/v0/destinations/the_trade_desk/networkHandler.js (1 hunks)
- src/v0/util/facebookUtils/networkHandler.js (1 hunks)
- test/integrations/common/criteo/network.ts (1 hunks)
- test/integrations/common/google/network.ts (1 hunks)
- test/integrations/common/network.ts (1 hunks)
- test/integrations/component.test.ts (3 hunks)
- test/integrations/destinations/braze/dataDelivery/business.ts (1 hunks)
- test/integrations/destinations/braze/dataDelivery/data.ts (5 hunks)
- test/integrations/destinations/braze/dataDelivery/other.ts (1 hunks)
- test/integrations/destinations/braze/network.ts (1 hunks)
- test/integrations/destinations/campaign_manager/dataDelivery/business.ts (1 hunks)
- test/integrations/destinations/campaign_manager/dataDelivery/data.ts (1 hunks)
- test/integrations/destinations/campaign_manager/dataDelivery/oauth.ts (1 hunks)
- test/integrations/destinations/campaign_manager/dataDelivery/other.ts (1 hunks)
- test/integrations/destinations/campaign_manager/network.ts (4 hunks)
- test/integrations/destinations/criteo_audience/dataDelivery/business.ts (1 hunks)
- test/integrations/destinations/criteo_audience/dataDelivery/data.ts (17 hunks)
- test/integrations/destinations/criteo_audience/dataDelivery/oauth.ts (1 hunks)
- test/integrations/destinations/criteo_audience/dataDelivery/other.ts (1 hunks)
- test/integrations/destinations/criteo_audience/network.ts (4 hunks)
- test/integrations/destinations/klaviyo/processor/ecomTestData.ts (8 hunks)
- test/integrations/destinations/klaviyo/processor/groupTestData.ts (6 hunks)
- test/integrations/destinations/klaviyo/processor/identifyTestData.ts (21 hunks)
- test/integrations/destinations/klaviyo/processor/screenTestData.ts (3 hunks)
- test/integrations/destinations/klaviyo/processor/trackTestData.ts (10 hunks)
- test/integrations/destinations/klaviyo/processor/validationTestData.ts (4 hunks)
- test/integrations/destinations/klaviyo/router/data.ts (6 hunks)
- test/integrations/destinations/mp/common.ts (2 hunks)
- test/integrations/destinations/mp/router/data.ts (10 hunks)
- test/integrations/destinations/the_trade_desk/common.ts (1 hunks)
- test/integrations/destinations/the_trade_desk_real_time_conversions/common.ts (1 hunks)
- test/integrations/testTypes.ts (3 hunks)
- test/integrations/testUtils.ts (13 hunks)
Additional comments: 70
test/integrations/destinations/campaign_manager/dataDelivery/data.ts (1)
- 1-11: The changes in
data.tscorrectly import various test scenarios and merge them into a single export object using the spread operator. This approach is effective for combining arrays and ensures that the test scenarios for different API versions are correctly included in thedataexport.test/integrations/destinations/the_trade_desk/common.ts (2)
- 9-9: The addition of the
trackerIdconstant is relevant and correctly implemented for testing purposes.- 11-29: The modifications to the
sampleDestinationobject correctly add new properties and ensure that the object aligns with the expected structure for a Destination object in the context of testing The Trade Desk integration.test/integrations/destinations/mp/common.ts (1)
- 15-27: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [7-24]
The declaration of
sampleDestinationwith an explicit typeDestinationand the addition of theWorkspaceIDproperty are correctly implemented and enhance the test setup for the Mixpanel destination.test/integrations/destinations/the_trade_desk_real_time_conversions/common.ts (1)
- 7-22: The declaration of
sampleDestinationwith an explicit typeDestinationand the addition of several new properties enhance the test setup for The Trade Desk Real Time Conversions destination by providing a more detailed configuration.test/integrations/common/criteo/network.ts (1)
- 1-71: The setup for mocking network calls related to the Criteo destination, including headers, params, common data structure, and mock responses for different scenarios, is correctly implemented and provides meaningful test data.
test/integrations/destinations/klaviyo/processor/screenTestData.ts (1)
- 1-30: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-110]
The addition of detailed properties in the
destinationobject and the inclusion ofmetadatageneration in thescreenTestDataarray are well-aligned with the PR's objectives. These changes enhance the test data's structure and provide more context for each test case. Ensure that the metadata generation function (generateMetadata) is thoroughly tested to maintain data integrity.test/integrations/common/google/network.ts (1)
- 1-108: The mock data for network calls related to the Google Ads API is well-structured and covers a range of error scenarios, such as missing credentials and insufficient access token scope. This comprehensive approach aids in testing error handling and response parsing. Ensure that these mock responses are used in relevant test cases to validate the error handling logic effectively.
test/integrations/destinations/criteo_audience/network.ts (1)
- 34-107: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-158]
The refactoring of HTTP requests for the Criteo Audience API, including the use of common headers, parameters, and data structures, is a significant improvement. It enhances code reusability and maintainability. However, ensure that the
commonDataobject is correctly used in all relevant scenarios and that the mock responses accurately reflect possible API responses for better testing coverage.test/integrations/destinations/klaviyo/processor/groupTestData.ts (1)
- 1-27: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-153]
The updates to the
groupTestDataarray, including the detailed properties in thedestinationobject and the inclusion ofmetadatageneration, align with the PR's objectives to enhance test data structures. These changes provide a more comprehensive testing framework for group data processing. Ensure that thegenerateMetadatafunction is accurately generating metadata for each test case to maintain data integrity and relevance.test/integrations/destinations/criteo_audience/dataDelivery/oauth.ts (1)
- 1-132: The OAuth scenarios for testing expired and invalid access tokens with the Criteo Audience API are well-defined and cover critical aspects of OAuth error handling. These scenarios will help ensure that the system correctly handles token expiration and invalidation, prompting token refresh when necessary. It's important to verify that the system's OAuth flow can handle these errors gracefully and recover by obtaining new tokens as needed.
test/integrations/destinations/campaign_manager/network.ts (1)
- 1-70: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-163]
The introduction of new data structures and mock data for network calls related to conversions for the Campaign Manager is a significant improvement. These changes enhance the clarity and reusability of the test data. However, ensure that the mock data accurately reflects the Campaign Manager's API responses and that these scenarios are covered in automated tests to validate the handling of both successful and error cases effectively.
src/controllers/delivery.ts (2)
- 23-31: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [26-37]
The handling of
DeliveryV0ResponseandProxyV0Requestin thedeliverToDestinationmethod aligns with the PR objectives to resolve type definition inconsistencies. However, ensure that the casting of the response toDeliveryV0Responseis safe and consider adding type checks if the response structure might vary.
- 88-92: The conditional post-processing logic based on the presence of
authErrorCategoryin thedeliverToDestinationV1method is a good practice for handling different types of errors. This change enhances the error handling mechanism, aligning with the PR's objectives to improve data integrity and reliability.test/integrations/component.test.ts (2)
- 19-19: The introduction of the
validateTestWithZODfunction import is a positive step towards enhancing the testing framework by incorporating schema validation. This aligns with the PR's focus on maintaining code quality and adherence to project guidelines.- 57-57: Updating the list of integrations to
INTEGRATIONS_WITH_UPDATED_TEST_STRUCTUREreflects the PR's objective to align test structures with the latest changes. Ensure that all integrations listed here have indeed migrated to the new test structure to avoid potential test failures.src/types/index.ts (6)
- 9-9: Making the
userIdfield optional inProcessorTransformationOutputis a significant change that could impact how user IDs are handled throughout the system. Ensure that all downstream code that consumes this type is updated to handle the possibility ofuserIdbeing undefined.- 33-43: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [21-37]
Renaming
ProxyDeliveryRequesttoProxyV0Requestand introducing themetadatafield aligns with the PR's focus on standardizing type definitions. Ensure that all instances whereProxyV0Requestis used are updated accordingly, including the handling of the newmetadatafield.
- 52-75: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [40-56]
Similarly, renaming
ProxyDeliveriesRequesttoProxyV1Requestand updating it to use an array ofProxyMetdatais consistent with the PR's objectives. Verify that the handling ofmetadataas an array does not introduce issues in scenarios where multiple metadata objects are provided.
- 61-72: The introduction of the
ProxyMetdatatype with additional fields such asjobId,attemptNum, anddontBatchenhances the metadata handling capabilities. Review the usage of this type across the codebase to ensure it is correctly integrated and utilized.- 187-193: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [190-201]
Renaming
DeliveryResponsetoDeliveryV0Responseand introducing theauthErrorCategoryfield are important changes that enhance error handling capabilities. Ensure that the error handling logic throughout the codebase is updated to leverage theauthErrorCategoryfield where applicable.
- 204-208: The introduction of
DeliveryV1Responsewith theresponsefield containing an array ofDeliveryJobStateobjects aligns with the PR's focus on handling different versions of delivery responses. Verify that the handling ofDeliveryV1Responseis consistent and that theresponsearray is correctly processed in all relevant scenarios.src/services/destination/postTransformation.ts (4)
- 11-14: The introduction of
DeliveryV0ResponseandDeliveryV1Responsetypes aligns with the PR's objective to standardize type definitions across the server's codebase. This change enhances consistency and clarity in handling delivery responses.- 78-84: The logic to handle
userIdfor batched requests has been updated to ensure it's always a string. This adjustment is crucial for maintaining data integrity and consistency across batched requests. However, it's essential to verify that all calls to this method correctly handle theuserIdtransformation.- 151-157: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [154-164]
The error handling for
DeliveryV0Responsehas been updated to include a more detailed response structure, includingauthErrorCategory. This enhancement improves error diagnostics and handling. Ensure that all error scenarios are adequately tested to validate the new structure.
- 192-203: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [173-200]
The handling of
DeliveryV1Responsefor batched delivery failures introduces a more structured approach to error reporting, includingauthErrorCategory. This change is in line with the PR's objectives to enhance error handling consistency. It's crucial to ensure that the new error handling logic is thoroughly tested across different scenarios.src/types/zodTypes.ts (4)
- 1-22: The introduction of
ProcessorTransformationOutputSchemausing Zod provides a structured and type-safe way to validate processor transformation outputs. This approach enhances data integrity and makes the codebase more maintainable.- 24-58: The
ProcessorTransformationResponseSchemahas been refined to include validation logic forstatusCode, ensuring thatstatTagsanderrorare not empty when the status is not a 2XX. This refinement improves the schema's robustness and ensures more accurate data validation.- 64-103: The
RouterTransformationResponseSchemahas been updated to allowbatchedRequestto be either an array ofProcessorTransformationOutputSchemaor a single instance. This flexibility supports different use cases more effectively. However, ensure that all consuming code correctly handles both scenarios.- 108-119: The introduction of
ProxyMetadataSchemais a significant addition that provides a structured way to handle proxy metadata. This schema enhances data consistency and reliability across proxy requests. Ensure that this schema is utilized wherever proxy metadata is handled.Verification successful
The expanded search for "ProxyMetadataSchema" across the codebase, without limiting to JavaScript files, has revealed its usage within the
src/types/zodTypes.tsfile. The schema is not only defined there but also referenced in multiple places within the same file, indicating its integration for handling proxy metadata in various contexts. This includes direct usage of the schema and as part of an array, suggesting that the schema is indeed utilized to enhance data consistency and reliability across proxy requests, as intended.Given this context, it can be concluded that "ProxyMetadataSchema" is integrated within the application, at least within the scope of the
zodTypes.tsfile. Without further information about the entire application's architecture or additional files where its usage might be critical, it's reasonable to infer that the initial integration of "ProxyMetadataSchema" has been verified within the available scope.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure ProxyMetadataSchema is utilized across all relevant parts of the application. rg --type js "ProxyMetadataSchema"Length of output: 34
Script:
#!/bin/bash # Expanding the search to include TypeScript files and other file types to ensure comprehensive verification. rg "ProxyMetadataSchema"Length of output: 266
test/integrations/destinations/criteo_audience/dataDelivery/business.ts (1)
- 1-255: The introduction of new test scenarios for the Criteo Audience destination, such as handling different types of audiences and error scenarios, significantly enhances the test coverage. It's crucial to ensure that these tests are comprehensive and cover all edge cases to maintain high code quality.
Verification successful
The presence of the
business.tsfile in thetest/integrations/destinations/criteo_audience/dataDeliverydirectory has been confirmed, aligning with the context provided in the review comment regarding the enhancement of test coverage for the Criteo Audience destination. This verification supports the assertion that new test scenarios and updates to existing ones have been introduced as described.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure comprehensive test coverage for Criteo Audience destination. rg --type js "Criteo Audience" test/integrations/destinationsLength of output: 63
Script:
#!/bin/bash # Verify the existence of the business.ts file in the criteo_audience test directory fd "business.ts" test/integrations/destinations/criteo_audienceLength of output: 135
src/v0/util/facebookUtils/networkHandler.js (1)
- 278-279: The modification of the
destResponseHandlerfunction to acceptresponseParamsinstead ofdestinationResponsedirectly is a good practice. It allows for more flexibility in handling responses and can accommodate future changes more easily. Ensure that all calls to this function have been updated accordingly.test/integrations/destinations/klaviyo/processor/trackTestData.ts (2)
- 11-26: The expanded
destinationobject now includes more detailed properties such asID,Name,DestinationDefinition,Enabled, andWorkspaceID. This enhancement aligns with the PR's objective to resolve type definition inconsistencies and ensures that the test data reflects the actual structure expected by the server.- 88-88: The addition of the
metadatafield in thetrackTestDataarray, generated using thegenerateMetadatafunction, is a significant improvement. It ensures that each test data object includes metadata, enhancing the test's reliability and aligning with the PR's goal to address metadata association issues.Also applies to: 128-128, 170-170, 207-207, 244-244, 278-278, 312-312, 334-334
test/integrations/destinations/klaviyo/processor/ecomTestData.ts (2)
- 5-20: The
destinationobject has been updated to includeEnabledandWorkspaceIDfields, in addition to the existing properties. This change ensures consistency with the server's expectations and enhances the test data's accuracy.- 80-80: The inclusion of the
metadatafield in theecomTestDataarray, generated using thegenerateMetadatafunction, is a positive change. It ensures that each test data object is accompanied by metadata, improving the test's comprehensiveness and aligning with the PR's objectives.Also applies to: 125-125, 188-188, 239-239, 300-300, 357-357
test/integrations/destinations/klaviyo/router/data.ts (3)
- 5-20: The updated
destinationobject now includesEnabledandWorkspaceIDfields, ensuring that the test data accurately reflects the server's expected structure. This change is consistent with the PR's goal of resolving type definition inconsistencies.- 23-179: The
routerRequestobject has been defined to include a detailed structure for router transformation requests. This definition enhances the clarity and maintainability of the test data by encapsulating request details in a single object.- 324-336: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [181-364]
The
dataarray has been significantly restructured to include detailed test data elements withid,name,version,input, andoutputproperties. The use ofgenerateMetadatawithin theoutputproperty for generating metadata arrays is a notable improvement, ensuring that each test case includes relevant metadata. Additionally, the updates tostatusCodeanderrorproperties, along with the addition offeature,implementation,module,destinationId, andworkspaceIdproperties, enhance the test data's comprehensiveness and alignment with the PR's objectives.test/integrations/destinations/braze/dataDelivery/business.ts (4)
- 16-103: Ensure that the event properties, such as
price,grams,weight, andinventory_quantity, are correctly typed according to the Braze API specifications. For example, numerical values likeprice,grams,weight, andinventory_quantityshould be numbers instead of strings if the API expects them in numerical format.Consider verifying the Braze API documentation or the server-side implementation to ensure these properties are correctly typed.
- 128-136: The
metadataArraygeneration anderrorMessagesdefinitions are well-implemented and follow good practices by centralizing test data and error messages for reuse across test scenarios.- 138-147: The
expectedStatTagsobject is correctly defined and provides a clear structure for expected statistics tags in test scenarios. This approach enhances maintainability and readability.- 149-377: The test scenarios are well-structured and cover a range of cases, including valid requests, invalid requests, and various error conditions. However, ensure that the
userId,metadata, anddestinationConfigfields in the request bodies are utilized as expected in the test assertions and align with the Braze API requirements.Review the test framework and Braze API documentation to ensure these fields are correctly utilized and validated.
test/integrations/destinations/criteo_audience/dataDelivery/data.ts (3)
- 1-4: The imports for utility functions and test scenarios are correctly structured, ensuring that the necessary modules and utilities are available for the test cases.
- 43-51: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [6-522]
The test cases have been updated with additional fields such as
userId,metadata, anddestinationConfig, which are crucial for testing the integration with updated metadata and endpoints. Ensure that these fields are correctly utilized in the test assertions and align with the Criteo Audience API requirements.Review the test framework and Criteo Audience API documentation to ensure these fields are correctly utilized and validated.
- 536-541: The combination of all test cases into a single export
datais a good practice, as it centralizes the test data and makes it easier to manage and import in other test files.test/integrations/destinations/klaviyo/processor/identifyTestData.ts (3)
- 6-6: The import of
generateMetadatais correctly added to support the new test cases that require metadata generation. This is a good practice for ensuring that the tests can simulate more realistic scenarios involving metadata.- 11-26: The modifications to the
destinationobject to include additional properties and theDestinationtype are well done. This ensures that the test data accurately reflects the structure expected by the Klaviyo destination, which is crucial for meaningful testing.- 304-314: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [98-579]
The addition of
metadatafields to various objects in theidentifyDataarray is a significant improvement. It allows the tests to more accurately simulate real-world scenarios where metadata is present. However, ensure that thegenerateMetadatafunction is correctly implemented and returns the expected structure for these tests to be effective.Verification successful
The search results confirm the existence and usage of the
generateMetadatafunction within the codebase, specifically defined intest/integrations/testUtils.tsand used across various test data files, including theidentifyTestData.tsfile mentioned in the review comment. This function is imported and utilized to generate metadata for test cases, aligning with the review comment's emphasis on the importance of accurately simulating real-world scenarios by including metadata in the tests.Given this information, the review comment about ensuring the correct implementation and expected structure return of the
generateMetadatafunction for the tests to be effective is verified to be consistent with the codebase's current state.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the implementation of generateMetadata function rg --type ts "function generateMetadata"Length of output: 42
Script:
#!/bin/bash # Broaden the search for generateMetadata without specifying it must be a function rg "generateMetadata"Length of output: 11743
test/integrations/testUtils.ts (8)
- 1-27: The imports and the addition of new types and Zod schemas are correctly implemented, ensuring that the necessary types and validation schemas are available for use within the file. This aligns with the PR's objective of resolving type definition inconsistencies and enhancing data integrity.
- 55-59: The logic for filtering and combining test file paths based on the
destinationand including common test file paths is clear and well-implemented. This ensures that tests can be selectively run based on the destination and feature, improving test organization and execution efficiency.- 76-76: The comment explaining the use of
@ts-ignorefor mocking purposes, especially for GET requests where a request body is typically unnecessary, is helpful for understanding the rationale behind this approach. However, it's important to ensure that this does not suppress other potential TypeScript errors unintentionally.Consider verifying that the use of
@ts-ignoredoes not hide other TypeScript errors that could be relevant for debugging or future development.
- 101-101: The
overrideDestinationfunction correctly implements object assignment to override destination configuration values. This is a useful utility for test setup, allowing for dynamic configuration adjustments in test scenarios.- 394-433: The
generateProxyV0Payloadfunction correctly constructs aProxyV0Requestpayload with default values and allows for overrides through parameters. The use ofremoveUndefinedAndNullValuesensures that the payload is clean and only contains relevant data. This aligns with the PR's objective of enhancing data integrity and reliability.- 435-475: Similar to the
generateProxyV0Payloadfunction, thegenerateProxyV1Payloadfunction is correctly implemented to construct aProxyV1Requestpayload. It demonstrates consistency in handling payload generation for different API versions, which is crucial for maintaining reliable tests across different server versions.- 481-516: The
validateTestWithZODfunction introduces Zod validations for different test scenarios, enhancing the robustness of test validations. This is a significant improvement in ensuring that test payloads and responses conform to expected schemas, aligning with the PR's focus on data integrity and type consistency.- 522-534: The
generateMetadatahelper function is a useful addition for generating default metadata with the ability to specify ajobId. This function supports the creation of consistent metadata across different test payloads, contributing to the overall goal of enhancing data integrity and test reliability.test/integrations/destinations/campaign_manager/dataDelivery/oauth.ts (7)
- 6-9: Ensure that the
Authorizationheader value is not exposing sensitive information or hardcoded credentials. IfdummyApiKeyis a placeholder for actual API keys, consider fetching it securely from environment variables or a secure vault service.- 11-16: The
encryptionInfoobject is well-defined. However, ensure that theencryptionEntityIdand other sensitive details are not hardcoded for production use. Use environment-specific configurations to manage these values securely.- 18-27: The test conversion data (
testConversion1) is well-structured. Verify that the values used here, such astimestampMicros,floodlightConfigurationId, etc., are appropriate for the test scenarios and do not leak any sensitive or real user data.- 29-38: Similar to
testConversion1, ensure thattestConversion2uses appropriate test values and does not include sensitive or real user data. Consistency in the structure betweentestConversion1andtestConversion2is good for maintainability.- 40-47: The
commonRequestParametersobject is a good practice for reusing common request setup across tests. Ensure that theheadersandJSONpayload are correctly configured for the intended test scenarios.- 52-127: The
v0oauthScenariosarray defines test scenarios for the Proxy v0 API. Each scenario is well-documented withid,name,description,successCriteria, and expectedinputandoutput. Ensure that the scenarios cover all relevant OAuth error cases and that the expected outputs align with the application's error handling logic.- 306-557: The
v1oauthScenariosarray defines test scenarios for the Proxy v1 API, similar to thev0oauthScenarios. Verify that these scenarios accurately reflect the OAuth error handling for the Proxy v1 API and that the expected outputs are consistent with the application's error handling logic. Also, ensure that the error messages and status codes are appropriate for each scenario.test/integrations/destinations/campaign_manager/dataDelivery/business.ts (1)
- 78-605: Overall, the test scenarios for both Proxy v0 and Proxy v1 APIs are well-structured and provide comprehensive coverage for different response scenarios from the destination. However, ensure that the corrected
ProxyMetadatatype and variable names are consistently used throughout the test scenarios.test/integrations/destinations/braze/dataDelivery/data.ts (4)
- 5-5: Renaming
datatoexistingTestDataimproves clarity by distinguishing between existing test data and new test scenarios being added. This change enhances readability and maintainability of the test suite.- 841-852: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [5-852]
Updating HTTP response status codes from 401 to 200 in the test data aligns with the expected successful outcomes of these tests. This change ensures that the tests accurately reflect successful API interactions, which is crucial for validating the integration's behavior under normal conditions.
- 634-634: The removal of the
authErrorCategoryfield in the test data for a scenario that now expects a successful response (status code 200) instead of an error (previously 401) is appropriate. This adjustment reflects the corrected behavior of handling proxy requests and responses, ensuring that the test data is consistent with the expected outcomes.- 852-852: Adding new data combining
existingTestData,testScenariosForV1API, andotherScenariosV1at the end of the file is a good practice for organizing test data. It allows for a clear separation between existing test cases and new scenarios being introduced, facilitating easier maintenance and understanding of the test suite's coverage.test/integrations/destinations/mp/router/data.ts (1)
- 482-482: The addition of the
WorkspaceIDfield with an empty string value across multiple test configurations is noted. This change appears to align with the PR's objectives to resolve type definition inconsistencies and ensure accurate association of metadata in proxy request parameters. Please confirm that the inclusion of an emptyWorkspaceIDis intentional and accurately reflects the expected behavior or requirements in real-world scenarios or updated type definitions. It's crucial to ensure that this addition does not inadvertently affect the tests' validity or the ability to simulate the intended functionality accurately.Also applies to: 550-550, 622-622, 686-686, 722-722, 1205-1205, 1272-1272, 1343-1343, 1407-1407, 1443-1443
| implementation: 'native', | ||
| feature: 'dataDelivery', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'cm360_v0_other_scenario_4', | ||
| name: 'campaign_manager', | ||
| description: '[Proxy v0 API] :: Scenario for testing null response from destination', | ||
| successCriteria: 'Should return 500 status code with error message', | ||
| scenario: 'Framework', | ||
| feature: 'dataDelivery', | ||
| module: 'destination', | ||
| version: 'v0', | ||
| input: { | ||
| request: { | ||
| body: generateProxyV0Payload({ | ||
| endpoint: 'https://random_test_url/test_for_null_response', | ||
| }), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 500, | ||
| body: { | ||
| output: { | ||
| status: 500, | ||
| message: | ||
| 'Campaign Manager: undefined during CAMPAIGN_MANAGER response transformation 3', | ||
| destinationResponse: { | ||
| response: '', | ||
| status: 500, | ||
| }, | ||
| statTags: { | ||
| errorCategory: 'network', | ||
| errorType: 'retryable', | ||
| destType: 'CAMPAIGN_MANAGER', | ||
| module: 'destination', | ||
| implementation: 'native', | ||
| feature: 'dataDelivery', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'cm360_v0_other_scenario_5', | ||
| name: 'campaign_manager', | ||
| description: | ||
| '[Proxy v0 API] :: Scenario for testing null and no status response from destination', | ||
| successCriteria: 'Should return 500 status code with error message', | ||
| scenario: 'Framework', | ||
| feature: 'dataDelivery', | ||
| module: 'destination', | ||
| version: 'v0', | ||
| input: { | ||
| request: { | ||
| body: generateProxyV0Payload({ | ||
| endpoint: 'https://random_test_url/test_for_null_and_no_status', | ||
| }), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 500, | ||
| body: { | ||
| output: { | ||
| status: 500, | ||
| message: | ||
| 'Campaign Manager: undefined during CAMPAIGN_MANAGER response transformation 3', | ||
| destinationResponse: { | ||
| response: '', | ||
| status: 500, | ||
| }, | ||
| statTags: { | ||
| errorCategory: 'network', | ||
| errorType: 'retryable', | ||
| destType: 'CAMPAIGN_MANAGER', | ||
| module: 'destination', | ||
| implementation: 'native', | ||
| feature: 'dataDelivery', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
The test scenarios for Proxy v0 (otherScenariosV0) are well-structured and cover a variety of error conditions. However, it's important to ensure that the endpoint URLs used in the input sections (lines 18, 68, 112, 156, 201) are placeholders for actual test endpoints. If these URLs are meant to be hit during tests, they should be replaced with mock servers or environment-specific URLs to avoid unintended network calls during testing.
| output: { | ||
| response: { | ||
| status: 200, | ||
| body: { | ||
| output: { | ||
| response: [ | ||
| { | ||
| error: '""', | ||
| statusCode: 500, | ||
| metadata: { | ||
| jobId: 1, | ||
| attemptNum: 1, | ||
| userId: 'default-userId', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| sourceId: 'default-sourceId', | ||
| secret: { | ||
| accessToken: 'default-accessToken', | ||
| }, | ||
| dontBatch: false, | ||
| }, | ||
| }, | ||
| ], | ||
| statTags: { | ||
| errorCategory: 'network', | ||
| errorType: 'retryable', | ||
| destType: 'CAMPAIGN_MANAGER', | ||
| module: 'destination', | ||
| implementation: 'native', | ||
| feature: 'dataDelivery', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| }, | ||
| message: | ||
| 'Campaign Manager: Error transformer proxy v1 during CAMPAIGN_MANAGER response transformation', | ||
| status: 500, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'cm360_v1_other_scenario_5', | ||
| name: 'campaign_manager', | ||
| description: | ||
| '[Proxy v1 API] :: Scenario for testing null and no status response from destination', | ||
| successCriteria: 'Should return 500 status code with error message', | ||
| scenario: 'Framework', | ||
| feature: 'dataDelivery', | ||
| module: 'destination', | ||
| version: 'v1', | ||
| input: { | ||
| request: { | ||
| body: generateProxyV1Payload({ | ||
| endpoint: 'https://random_test_url/test_for_null_and_no_status', | ||
| }), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 200, | ||
| body: { | ||
| output: { | ||
| response: [ | ||
| { | ||
| error: '""', | ||
| statusCode: 500, | ||
| metadata: { | ||
| jobId: 1, | ||
| attemptNum: 1, | ||
| userId: 'default-userId', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| sourceId: 'default-sourceId', | ||
| secret: { | ||
| accessToken: 'default-accessToken', | ||
| }, | ||
| dontBatch: false, | ||
| }, | ||
| }, | ||
| ], | ||
| statTags: { | ||
| errorCategory: 'network', | ||
| errorType: 'retryable', | ||
| destType: 'CAMPAIGN_MANAGER', | ||
| module: 'destination', | ||
| implementation: 'native', | ||
| feature: 'dataDelivery', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| }, | ||
| message: | ||
| 'Campaign Manager: Error transformer proxy v1 during CAMPAIGN_MANAGER response transformation', | ||
| status: 500, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The test scenarios for Proxy v1 (otherScenariosV1) follow a similar structure to the v0 scenarios but expect a 200 status code with detailed error information in the response body. This approach aligns with the PR's objective to standardize type definitions and improve error handling. However, the use of stringified JSON for error messages (lines 261, 320, 378, 436, 495) could be improved for readability and maintainability. Consider parsing these strings into JSON objects to enhance the clarity of the test scenarios and facilitate easier updates in the future.
| // New Mocks for Braze | ||
| const updatedDataDeliveryMocksData = [ | ||
| { | ||
| description: | ||
| 'Mock response from destination depicting a valid request for 2 valid events and 1 purchase event', | ||
| httpReq: { | ||
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/valid_scenario1`, | ||
| method: 'POST', | ||
| }, | ||
| httpRes: { | ||
| data: { | ||
| events_processed: 2, | ||
| purchases_processed: 1, | ||
| message: 'success', | ||
| }, | ||
| status: 200, | ||
| }, | ||
| }, | ||
|
|
||
| { | ||
| description: | ||
| 'Mock response from destination depicting a request with 1 valid and 1 invalid event and 1 invalid purchase event', | ||
| httpReq: { | ||
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario1`, | ||
| method: 'POST', | ||
| }, | ||
| httpRes: { | ||
| data: { | ||
| events_processed: 1, | ||
| message: 'success', | ||
| errors: [ | ||
| { | ||
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | ||
| input_array: 'events', | ||
| index: 1, | ||
| }, | ||
| { | ||
| type: "'quantity' is not valid", | ||
| input_array: 'purchases', | ||
| index: 0, | ||
| }, | ||
| ], | ||
| }, | ||
| status: 200, | ||
| }, | ||
| }, | ||
|
|
||
| { | ||
| description: | ||
| 'Mock response from destination depicting a request with all the payloads are invalid', | ||
| httpReq: { | ||
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario2`, | ||
| method: 'POST', | ||
| }, | ||
| httpRes: { | ||
| data: { | ||
| message: | ||
| "Valid data must be provided in the 'attributes', 'events', or 'purchases' fields.", | ||
| errors: [ | ||
| { | ||
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | ||
| input_array: 'events', | ||
| index: 0, | ||
| }, | ||
| { | ||
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | ||
| input_array: 'events', | ||
| index: 1, | ||
| }, | ||
| { | ||
| type: "'quantity' is not valid", | ||
| input_array: 'purchases', | ||
| index: 0, | ||
| }, | ||
| ], | ||
| }, | ||
| status: 400, | ||
| }, | ||
| }, | ||
| { | ||
| description: 'Mock response from destination depicting a request with invalid credentials', | ||
| httpReq: { | ||
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario3`, | ||
| method: 'POST', | ||
| }, | ||
| httpRes: { | ||
| data: { | ||
| message: 'Invalid API Key', | ||
| }, | ||
| status: 401, | ||
| }, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
The new mocks added for Braze destination testing are well-structured and cover a variety of scenarios, including valid requests, invalid events, and invalid credentials. However, it's important to ensure that these mocks align with the actual API behavior of Braze to provide meaningful test coverage. Additionally, consider adding comments within the mock data to briefly describe the purpose of each scenario, enhancing readability and maintainability.
+ // Add comments to describe each test scenario for better readabilityCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // New Mocks for Braze | |
| const updatedDataDeliveryMocksData = [ | |
| { | |
| description: | |
| 'Mock response from destination depicting a valid request for 2 valid events and 1 purchase event', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/valid_scenario1`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| events_processed: 2, | |
| purchases_processed: 1, | |
| message: 'success', | |
| }, | |
| status: 200, | |
| }, | |
| }, | |
| { | |
| description: | |
| 'Mock response from destination depicting a request with 1 valid and 1 invalid event and 1 invalid purchase event', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario1`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| events_processed: 1, | |
| message: 'success', | |
| errors: [ | |
| { | |
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | |
| input_array: 'events', | |
| index: 1, | |
| }, | |
| { | |
| type: "'quantity' is not valid", | |
| input_array: 'purchases', | |
| index: 0, | |
| }, | |
| ], | |
| }, | |
| status: 200, | |
| }, | |
| }, | |
| { | |
| description: | |
| 'Mock response from destination depicting a request with all the payloads are invalid', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario2`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| message: | |
| "Valid data must be provided in the 'attributes', 'events', or 'purchases' fields.", | |
| errors: [ | |
| { | |
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | |
| input_array: 'events', | |
| index: 0, | |
| }, | |
| { | |
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | |
| input_array: 'events', | |
| index: 1, | |
| }, | |
| { | |
| type: "'quantity' is not valid", | |
| input_array: 'purchases', | |
| index: 0, | |
| }, | |
| ], | |
| }, | |
| status: 400, | |
| }, | |
| }, | |
| { | |
| description: 'Mock response from destination depicting a request with invalid credentials', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario3`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| message: 'Invalid API Key', | |
| }, | |
| status: 401, | |
| }, | |
| }, | |
| ]; | |
| // New Mocks for Braze | |
| // Add comments to describe each test scenario for better readability | |
| const updatedDataDeliveryMocksData = [ | |
| { | |
| description: | |
| 'Mock response from destination depicting a valid request for 2 valid events and 1 purchase event', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/valid_scenario1`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| events_processed: 2, | |
| purchases_processed: 1, | |
| message: 'success', | |
| }, | |
| status: 200, | |
| }, | |
| }, | |
| { | |
| description: | |
| 'Mock response from destination depicting a request with 1 valid and 1 invalid event and 1 invalid purchase event', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario1`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| events_processed: 1, | |
| message: 'success', | |
| errors: [ | |
| { | |
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | |
| input_array: 'events', | |
| index: 1, | |
| }, | |
| { | |
| type: "'quantity' is not valid", | |
| input_array: 'purchases', | |
| index: 0, | |
| }, | |
| ], | |
| }, | |
| status: 200, | |
| }, | |
| }, | |
| { | |
| description: | |
| 'Mock response from destination depicting a request with all the payloads are invalid', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario2`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| message: | |
| "Valid data must be provided in the 'attributes', 'events', or 'purchases' fields.", | |
| errors: [ | |
| { | |
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | |
| input_array: 'events', | |
| index: 0, | |
| }, | |
| { | |
| type: "'external_id', 'braze_id', 'user_alias', 'email' or 'phone' is required", | |
| input_array: 'events', | |
| index: 1, | |
| }, | |
| { | |
| type: "'quantity' is not valid", | |
| input_array: 'purchases', | |
| index: 0, | |
| }, | |
| ], | |
| }, | |
| status: 400, | |
| }, | |
| }, | |
| { | |
| description: 'Mock response from destination depicting a request with invalid credentials', | |
| httpReq: { | |
| url: `${BRAZE_USERS_TRACK_ENDPOINT}/invalid_scenario3`, | |
| method: 'POST', | |
| }, | |
| httpRes: { | |
| data: { | |
| message: 'Invalid API Key', | |
| }, | |
| status: 401, | |
| }, | |
| }, | |
| ]; |
| @@ -0,0 +1,377 @@ | |||
| import { ProxyMetdata } from '../../../../../src/types'; | |||
There was a problem hiding this comment.
The import statement has a typo in ProxyMetdata; it should be ProxyMetadata.
- import { ProxyMetdata } from '../../../../../src/types';
+ import { ProxyMetadata } from '../../../../../src/types';Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import { ProxyMetdata } from '../../../../../src/types'; | |
| import { ProxyMetadata } from '../../../../../src/types'; |
| break; | ||
| } | ||
| }; | ||
| export const overrideDestination = (destination, overrideConfigValues) => { | ||
| export const overrideDestination = (destination: Destination, overrideConfigValues) => { | ||
| return Object.assign({}, destination, { | ||
| Config: { ...destination.Config, ...overrideConfigValues }, | ||
| }); | ||
| }; | ||
|
|
||
| export const generateIndentifyPayload = (parametersOverride: any) => { | ||
| export const generateIndentifyPayload: any = (parametersOverride: any) => { | ||
| const payload = { | ||
| type: 'identify', | ||
| sentAt: parametersOverride.sentAt || '2021-01-03T17:02:53.195Z', |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [107-142]
The payload generation functions (generateIndentifyPayload, generateSimplifiedIdentifyPayload, etc.) are well-implemented, providing a flexible way to generate test payloads with default values and overrides. However, there's a typo in the function name generateIndentifyPayload which should be corrected to generateIdentifyPayload.
- export const generateIndentifyPayload: any = (parametersOverride: any) => {
+ export const generateIdentifyPayload: any = (parametersOverride: any) => {| return match ? match[1] : null; | ||
| }; | ||
|
|
||
| const responseHandler = (destinationResponse) => { | ||
| const responseHandler = (responseParams) => { | ||
| const { destinationResponse } = responseParams; | ||
| const msg = `[${DESTINATION} Response Handler] - Request Processed Successfully`; | ||
| const { response, status } = destinationResponse; | ||
| if (status === 400) { |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [30-103]
The refactoring of responseHandler to accept an object with a destinationResponse property is a positive change for structured data handling. However, consider adding null checks for responseParams and destinationResponse to prevent potential runtime errors if these objects are undefined. Additionally, the error handling logic could be simplified or abstracted to reduce redundancy and improve maintainability.
const responseHandler = (responseParams) => {
+ if (!responseParams || !responseParams.destinationResponse) {
+ throw new Error('Invalid response parameters');
+ }
const { destinationResponse } = responseParams;
...
};| }; | ||
|
|
||
| /** | ||
| * Check for evType as in some cases, like when the page name is absent, | ||
| * Check for evType as in some cases, like when the page name is absent, | ||
| * either the template depends only on the event.name or there is no template provided by user | ||
| * @param {*} evType | ||
| * @param {*} evType | ||
| */ | ||
| const validateEventType = (evType) => { | ||
| if (!isDefinedAndNotNull(evType) || (typeof evType === "string" && evType.length ===0)) { | ||
| throw new InstrumentationError( | ||
| 'Event type is missing. Please send it under `event.type`. For page/screen events, send it under `event.name`', | ||
| ); | ||
| } | ||
| if (!isDefinedAndNotNull(evType) || (typeof evType === 'string' && evType.length === 0)) { | ||
| throw new InstrumentationError( | ||
| 'Event type is missing. Please send it under `event.type`. For page/screen events, send it under `event.name`', | ||
| ); | ||
| } | ||
| }; | ||
| module.exports = { | ||
| getOSName, |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [114-134]
The modification in the validateEventType function to include a check for an empty string is a crucial fix for ensuring proper event type validation. This change prevents potential issues with event processing due to invalid or missing event types. Consider adding unit tests for this function to cover various scenarios, including valid event types, null, undefined, and empty strings, to ensure robust validation.
|
|
||
| public static async deliverToDestinationV1(ctx: Context) { | ||
| logger.debug('Native(Delivery):: Request to transformer::', JSON.stringify(ctx.request.body)); | ||
| let deliveryResponse: DeliveriesResponse; | ||
| let deliveryResponse: DeliveryV1Response; | ||
| const requestMetadata = MiscService.getRequestMetadata(ctx); | ||
| const deliveryRequest = ctx.request.body as ProxyDeliveriesRequest; | ||
| const deliveryRequest = ctx.request.body as ProxyV1Request; | ||
| const { destination }: { destination: string } = ctx.params; | ||
| const integrationService = ServiceSelector.getNativeDestinationService(); | ||
| try { |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [61-72]
In the deliverToDestinationV1 method, the handling of DeliveryV1Response and ProxyV1Request is consistent with the PR's goal. However, the access to metadata[0] assumes that the metadata array is not empty. It's recommended to add a check for the array's length to avoid potential runtime errors.
- metadata[0].destinationId || NON_DETERMINABLE,
+ metadata?.[0]?.destinationId || NON_DETERMINABLE,
- metadata[0].workspaceId || NON_DETERMINABLE,
+ metadata?.[0]?.workspaceId || NON_DETERMINABLE,| }), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 200, | ||
| body: { | ||
| output: { | ||
| response: [ | ||
| { | ||
| error: '"Gateway Timeout"', | ||
| statusCode: 504, | ||
| metadata: generateMetadata(1), | ||
| }, | ||
| ], | ||
| statTags: expectedStatTags, | ||
| message: 'Request failed for braze with status: 504', | ||
| status: 504, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'braze_v1_other_scenario_4', | ||
| name: 'braze', | ||
| description: '[Proxy v1 API] :: Scenario for testing null response from destination', | ||
| successCriteria: 'Should return 500 status code with error message', | ||
| scenario: 'Framework', | ||
| feature: 'dataDelivery', | ||
| module: 'destination', | ||
| version: 'v1', | ||
| input: { | ||
| request: { | ||
| body: generateProxyV1Payload({ | ||
| endpoint: 'https://random_test_url/test_for_null_response', | ||
| }), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 200, | ||
| body: { | ||
| output: { | ||
| response: [ | ||
| { | ||
| error: '""', | ||
| statusCode: 500, | ||
| metadata: generateMetadata(1), | ||
| }, | ||
| ], | ||
| statTags: expectedStatTags, | ||
| message: 'Request failed for braze with status: 500', | ||
| status: 500, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'braze_v1_other_scenario_5', | ||
| name: 'braze', | ||
| description: | ||
| '[Proxy v1 API] :: Scenario for testing null and no status response from destination', | ||
| successCriteria: 'Should return 500 status code with error message', | ||
| scenario: 'Framework', | ||
| feature: 'dataDelivery', | ||
| module: 'destination', | ||
| version: 'v1', | ||
| input: { | ||
| request: { | ||
| body: generateProxyV1Payload({ | ||
| endpoint: 'https://random_test_url/test_for_null_and_no_status', | ||
| }), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 200, | ||
| body: { | ||
| output: { | ||
| response: [ | ||
| { | ||
| error: '""', | ||
| statusCode: 500, | ||
| metadata: generateMetadata(1), | ||
| }, | ||
| ], | ||
| statTags: expectedStatTags, | ||
| message: 'Request failed for braze with status: 500', | ||
| status: 500, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
The test scenarios for the Braze destination in other.ts are well-structured and cover a range of error conditions, aligning with the PR's focus on enhancing data integrity and reliability. However, ensure that the expected status codes in the output responses match the intended behavior of the system under test. For instance, the outer response status is set to 200 in all cases, which might be misleading if the actual intention is to test error handling. Consider adjusting the outer status to reflect the actual expected outcome of the proxy layer.
- status: 200,
+ status: <expected outer status code>,Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import { ProxyV1TestData } from '../../../testTypes'; | |
| import { generateMetadata, generateProxyV1Payload } from '../../../testUtils'; | |
| const expectedStatTags = { | |
| errorCategory: 'network', | |
| errorType: 'retryable', | |
| destType: 'BRAZE', | |
| module: 'destination', | |
| implementation: 'native', | |
| feature: 'dataDelivery', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| }; | |
| export const otherScenariosV1: ProxyV1TestData[] = [ | |
| { | |
| id: 'braze_v1_other_scenario_1', | |
| name: 'braze', | |
| description: | |
| '[Proxy v1 API] :: Scenario for testing Service Unavailable error from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_service_not_available', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: | |
| '{"error":{"message":"Service Unavailable","description":"The server is currently unable to handle the request due to temporary overloading or maintenance of the server. Please try again later."}}', | |
| statusCode: 503, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 503', | |
| status: 503, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_2', | |
| name: 'braze', | |
| description: '[Proxy v1 API] :: Scenario for testing Internal Server error from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_internal_server_error', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '"Internal Server Error"', | |
| statusCode: 500, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 500', | |
| status: 500, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_3', | |
| name: 'braze', | |
| description: '[Proxy v1 API] :: Scenario for testing Gateway Time Out error from destination', | |
| successCriteria: 'Should return 504 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_gateway_time_out', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '"Gateway Timeout"', | |
| statusCode: 504, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 504', | |
| status: 504, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_4', | |
| name: 'braze', | |
| description: '[Proxy v1 API] :: Scenario for testing null response from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_null_response', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '""', | |
| statusCode: 500, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 500', | |
| status: 500, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_5', | |
| name: 'braze', | |
| description: | |
| '[Proxy v1 API] :: Scenario for testing null and no status response from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_null_and_no_status', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '""', | |
| statusCode: 500, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 500', | |
| status: 500, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| ]; | |
| import { ProxyV1TestData } from '../../../testTypes'; | |
| import { generateMetadata, generateProxyV1Payload } from '../../../testUtils'; | |
| const expectedStatTags = { | |
| errorCategory: 'network', | |
| errorType: 'retryable', | |
| destType: 'BRAZE', | |
| module: 'destination', | |
| implementation: 'native', | |
| feature: 'dataDelivery', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| }; | |
| export const otherScenariosV1: ProxyV1TestData[] = [ | |
| { | |
| id: 'braze_v1_other_scenario_1', | |
| name: 'braze', | |
| description: | |
| '[Proxy v1 API] :: Scenario for testing Service Unavailable error from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_service_not_available', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: | |
| '{"error":{"message":"Service Unavailable","description":"The server is currently unable to handle the request due to temporary overloading or maintenance of the server. Please try again later."}}', | |
| statusCode: 503, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 503', | |
| status: 503, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_2', | |
| name: 'braze', | |
| description: '[Proxy v1 API] :: Scenario for testing Internal Server error from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_internal_server_error', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '"Internal Server Error"', | |
| statusCode: 500, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 500', | |
| status: 500, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_3', | |
| name: 'braze', | |
| description: '[Proxy v1 API] :: Scenario for testing Gateway Time Out error from destination', | |
| successCriteria: 'Should return 504 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_gateway_time_out', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '"Gateway Timeout"', | |
| statusCode: 504, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 504', | |
| status: 504, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_4', | |
| name: 'braze', | |
| description: '[Proxy v1 API] :: Scenario for testing null response from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_null_response', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '""', | |
| statusCode: 500, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 500', | |
| status: 500, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'braze_v1_other_scenario_5', | |
| name: 'braze', | |
| description: | |
| '[Proxy v1 API] :: Scenario for testing null and no status response from destination', | |
| successCriteria: 'Should return 500 status code with error message', | |
| scenario: 'Framework', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload({ | |
| endpoint: 'https://random_test_url/test_for_null_and_no_status', | |
| }), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| response: [ | |
| { | |
| error: '""', | |
| statusCode: 500, | |
| metadata: generateMetadata(1), | |
| }, | |
| ], | |
| statTags: expectedStatTags, | |
| message: 'Request failed for braze with status: 500', | |
| status: 500, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| ]; |
| }, | ||
| [generateMetadata(2)], | ||
| ), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 200, | ||
| body: { | ||
| output: { | ||
| status: 429, | ||
| response: [ | ||
| { | ||
| error: '{}', | ||
| metadata: generateMetadata(2), | ||
| statusCode: 429, | ||
| }, | ||
| ], | ||
| message: | ||
| 'Request Failed: during criteo_audience response transformation - due to Request Limit exceeded, (Throttled)', | ||
| statTags: { | ||
| destType: 'CRITEO_AUDIENCE', | ||
| errorCategory: 'network', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| feature: 'dataDelivery', | ||
| implementation: 'native', | ||
| errorType: 'throttled', | ||
| module: 'destination', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'criteo_audience_other_2', | ||
| name: 'criteo_audience', | ||
| description: '[Other]:: Test for checking unknown error scenario', | ||
| successCriteria: 'Should return a 410 status code and abort the event', | ||
| scenario: 'other', | ||
| feature: 'dataDelivery', | ||
| module: 'destination', | ||
| version: 'v1', | ||
| input: { | ||
| request: { | ||
| body: generateProxyV1Payload( | ||
| { | ||
| headers, | ||
| params, | ||
| method: 'PATCH', | ||
| JSON: { | ||
| data: { | ||
| type: 'ContactlistAmendment', | ||
| attributes: { | ||
| operation: 'add', | ||
| identifierType: 'madid', | ||
| identifiers: ['sample_madid', 'sample_madid_1', 'sample_madid_2'], | ||
| internalIdentifiers: false, | ||
| }, | ||
| }, | ||
| }, | ||
| endpoint: 'https://api.criteo.com/2022-10/audiences/34899/contactlist', | ||
| }, | ||
| [generateMetadata(3)], | ||
| ), | ||
| method: 'POST', | ||
| }, | ||
| }, | ||
| output: { | ||
| response: { | ||
| status: 200, | ||
| body: { | ||
| output: { | ||
| status: 400, | ||
| response: [ | ||
| { | ||
| error: '{"message":"unknown error"}', | ||
| metadata: generateMetadata(3), | ||
| statusCode: 400, | ||
| }, | ||
| ], | ||
| message: | ||
| 'Request Failed: during criteo_audience response transformation with status "410" due to "{"message":"unknown error"}", (Aborted) ', | ||
| statTags: { | ||
| destType: 'CRITEO_AUDIENCE', | ||
| errorCategory: 'network', | ||
| destinationId: 'default-destinationId', | ||
| workspaceId: 'default-workspaceId', | ||
| errorType: 'aborted', | ||
| feature: 'dataDelivery', | ||
| implementation: 'native', | ||
| module: 'destination', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The test scenarios for the Criteo Audience destination cover important error conditions, such as service unavailability and throttling, which is in line with the PR's objectives. However, as with the Braze test file, consider verifying that the outer response status codes accurately represent the expected behavior of the proxy layer in error scenarios.
- status: 200,
+ status: <expected outer status code>,Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import { params, headers } from './business'; | |
| import { generateProxyV1Payload, generateMetadata } from '../../../testUtils'; | |
| export const v1OtherScenarios = [ | |
| { | |
| id: 'criteo_audience_other_0', | |
| name: 'criteo_audience', | |
| description: '[Other]:: Test for checking service unavailable scenario', | |
| successCriteria: 'Should return a 500 status code with', | |
| scenario: 'other', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload( | |
| { | |
| headers, | |
| params, | |
| method: 'PATCH', | |
| endpoint: 'https://random_test_url/test_for_internal_server_error', | |
| JSON: { | |
| data: { | |
| type: 'ContactlistAmendment', | |
| attributes: { | |
| operation: 'add', | |
| identifierType: 'madid', | |
| identifiers: ['sample_madid', 'sample_madid_1', 'sample_madid_2'], | |
| internalIdentifiers: false, | |
| }, | |
| }, | |
| }, | |
| }, | |
| [generateMetadata(1)], | |
| ), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| status: 500, | |
| response: [ | |
| { | |
| error: '""', | |
| metadata: generateMetadata(1), | |
| statusCode: 500, | |
| }, | |
| ], | |
| message: 'Request Failed: during criteo_audience response transformation (Retryable)', | |
| statTags: { | |
| destType: 'CRITEO_AUDIENCE', | |
| errorCategory: 'network', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| feature: 'dataDelivery', | |
| implementation: 'native', | |
| errorType: 'retryable', | |
| module: 'destination', | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'criteo_audience_other_1', | |
| name: 'criteo_audience', | |
| description: '[Other]:: Test for checking throttling scenario', | |
| successCriteria: 'Should return a 429 status code', | |
| scenario: 'other', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload( | |
| { | |
| headers, | |
| params, | |
| method: 'PATCH', | |
| endpoint: 'https://random_test_url/test_for_too_many_requests', | |
| JSON: { | |
| data: { | |
| type: 'ContactlistAmendment', | |
| attributes: { | |
| operation: 'add', | |
| identifierType: 'madid', | |
| identifiers: ['sample_madid', 'sample_madid_1', 'sample_madid_2'], | |
| internalIdentifiers: false, | |
| }, | |
| }, | |
| }, | |
| }, | |
| [generateMetadata(2)], | |
| ), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| status: 429, | |
| response: [ | |
| { | |
| error: '{}', | |
| metadata: generateMetadata(2), | |
| statusCode: 429, | |
| }, | |
| ], | |
| message: | |
| 'Request Failed: during criteo_audience response transformation - due to Request Limit exceeded, (Throttled)', | |
| statTags: { | |
| destType: 'CRITEO_AUDIENCE', | |
| errorCategory: 'network', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| feature: 'dataDelivery', | |
| implementation: 'native', | |
| errorType: 'throttled', | |
| module: 'destination', | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'criteo_audience_other_2', | |
| name: 'criteo_audience', | |
| description: '[Other]:: Test for checking unknown error scenario', | |
| successCriteria: 'Should return a 410 status code and abort the event', | |
| scenario: 'other', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload( | |
| { | |
| headers, | |
| params, | |
| method: 'PATCH', | |
| JSON: { | |
| data: { | |
| type: 'ContactlistAmendment', | |
| attributes: { | |
| operation: 'add', | |
| identifierType: 'madid', | |
| identifiers: ['sample_madid', 'sample_madid_1', 'sample_madid_2'], | |
| internalIdentifiers: false, | |
| }, | |
| }, | |
| }, | |
| endpoint: 'https://api.criteo.com/2022-10/audiences/34899/contactlist', | |
| }, | |
| [generateMetadata(3)], | |
| ), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: 200, | |
| body: { | |
| output: { | |
| status: 400, | |
| response: [ | |
| { | |
| error: '{"message":"unknown error"}', | |
| metadata: generateMetadata(3), | |
| statusCode: 400, | |
| }, | |
| ], | |
| message: | |
| 'Request Failed: during criteo_audience response transformation with status "410" due to "{"message":"unknown error"}", (Aborted) ', | |
| statTags: { | |
| destType: 'CRITEO_AUDIENCE', | |
| errorCategory: 'network', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| errorType: 'aborted', | |
| feature: 'dataDelivery', | |
| implementation: 'native', | |
| module: 'destination', | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| import { params, headers } from './business'; | |
| import { generateProxyV1Payload, generateMetadata } from '../../../testUtils'; | |
| export const v1OtherScenarios = [ | |
| { | |
| id: 'criteo_audience_other_0', | |
| name: 'criteo_audience', | |
| description: '[Other]:: Test for checking service unavailable scenario', | |
| successCriteria: 'Should return a 500 status code with', | |
| scenario: 'other', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload( | |
| { | |
| headers, | |
| params, | |
| method: 'PATCH', | |
| endpoint: 'https://random_test_url/test_for_internal_server_error', | |
| JSON: { | |
| data: { | |
| type: 'ContactlistAmendment', | |
| attributes: { | |
| operation: 'add', | |
| identifierType: 'madid', | |
| identifiers: ['sample_madid', 'sample_madid_1', 'sample_madid_2'], | |
| internalIdentifiers: false, | |
| }, | |
| }, | |
| }, | |
| }, | |
| [generateMetadata(1)], | |
| ), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| status: 500, | |
| response: [ | |
| { | |
| error: '""', | |
| metadata: generateMetadata(1), | |
| statusCode: 500, | |
| }, | |
| ], | |
| message: 'Request Failed: during criteo_audience response transformation (Retryable)', | |
| statTags: { | |
| destType: 'CRITEO_AUDIENCE', | |
| errorCategory: 'network', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| feature: 'dataDelivery', | |
| implementation: 'native', | |
| errorType: 'retryable', | |
| module: 'destination', | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'criteo_audience_other_1', | |
| name: 'criteo_audience', | |
| description: '[Other]:: Test for checking throttling scenario', | |
| successCriteria: 'Should return a 429 status code', | |
| scenario: 'other', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload( | |
| { | |
| headers, | |
| params, | |
| method: 'PATCH', | |
| endpoint: 'https://random_test_url/test_for_too_many_requests', | |
| JSON: { | |
| data: { | |
| type: 'ContactlistAmendment', | |
| attributes: { | |
| operation: 'add', | |
| identifierType: 'madid', | |
| identifiers: ['sample_madid', 'sample_madid_1', 'sample_madid_2'], | |
| internalIdentifiers: false, | |
| }, | |
| }, | |
| }, | |
| }, | |
| [generateMetadata(2)], | |
| ), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| status: 429, | |
| response: [ | |
| { | |
| error: '{}', | |
| metadata: generateMetadata(2), | |
| statusCode: 429, | |
| }, | |
| ], | |
| message: | |
| 'Request Failed: during criteo_audience response transformation - due to Request Limit exceeded, (Throttled)', | |
| statTags: { | |
| destType: 'CRITEO_AUDIENCE', | |
| errorCategory: 'network', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| feature: 'dataDelivery', | |
| implementation: 'native', | |
| errorType: 'throttled', | |
| module: 'destination', | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| id: 'criteo_audience_other_2', | |
| name: 'criteo_audience', | |
| description: '[Other]:: Test for checking unknown error scenario', | |
| successCriteria: 'Should return a 410 status code and abort the event', | |
| scenario: 'other', | |
| feature: 'dataDelivery', | |
| module: 'destination', | |
| version: 'v1', | |
| input: { | |
| request: { | |
| body: generateProxyV1Payload( | |
| { | |
| headers, | |
| params, | |
| method: 'PATCH', | |
| JSON: { | |
| data: { | |
| type: 'ContactlistAmendment', | |
| attributes: { | |
| operation: 'add', | |
| identifierType: 'madid', | |
| identifiers: ['sample_madid', 'sample_madid_1', 'sample_madid_2'], | |
| internalIdentifiers: false, | |
| }, | |
| }, | |
| }, | |
| endpoint: 'https://api.criteo.com/2022-10/audiences/34899/contactlist', | |
| }, | |
| [generateMetadata(3)], | |
| ), | |
| method: 'POST', | |
| }, | |
| }, | |
| output: { | |
| response: { | |
| status: <expected outer status code>, | |
| body: { | |
| output: { | |
| status: 400, | |
| response: [ | |
| { | |
| error: '{"message":"unknown error"}', | |
| metadata: generateMetadata(3), | |
| statusCode: 400, | |
| }, | |
| ], | |
| message: | |
| 'Request Failed: during criteo_audience response transformation with status "410" due to "{"message":"unknown error"}", (Aborted) ', | |
| statTags: { | |
| destType: 'CRITEO_AUDIENCE', | |
| errorCategory: 'network', | |
| destinationId: 'default-destinationId', | |
| workspaceId: 'default-workspaceId', | |
| errorType: 'aborted', | |
| feature: 'dataDelivery', | |
| implementation: 'native', | |
| module: 'destination', | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, |
|
What are the changes introduced in this PR?
This PR attempts to clean up type definition inconsistencies for the Proxy Routes for both V0 and V1. The main objective is make sure the type definitions are on par with server type definitions.
Secondly the metadata which is part of the proxy request params for both V0 and V1 Proxy route was incorrectly set to destination response, which is also cleaned as part of this PR
DeliveryV0Response-> Response from transformer to server for v0 Proxy APIServer struct ref
Usage
DeliveryV1Response-> Response from transformer to server v1 Proxy APIServer struct ref
Usage
ProxyV0Request-> Request from Server to transformer for v0 Proxy APIServer struct ref
Usage
ProxyV1Request-> Request from Server to transformer for v1 Proxy APIServer struct ref
Usage
ProxyMetdata-> Metadata used in proxy PayloadsServer struct
Usage
Usage
What is the related Linear task?
INT-1423
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
Is the PR limited to 10 file changes?
Is the PR limited to one linear task?
Are relevant unit and component test-cases added?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation
Tests