fix: bug fix for track event #2852
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 changes involve updates to the Changes
Assessment against linked issues
Poem
TipsChat with CodeRabbit Bot (
|
![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: 4
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/v0/destinations/sfmc/transform.js (4 hunks)
Additional comments: 1
src/v0/destinations/sfmc/transform.js (1)
- 3-8: The addition of
isDefinedAndNotNullandisEmptyto the imports suggests that these utility functions will be used for validation checks in the code.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2852 +/- ##
========================================
Coverage 87.16% 87.17%
========================================
Files 772 772
Lines 28788 28794 +6
Branches 6764 6767 +3
========================================
+ Hits 25094 25100 +6
Misses 3351 3351
Partials 343 343 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/v0/destinations/sfmc/transform.js (4 hunks)
Additional comments: 2
src/v0/destinations/sfmc/transform.js (2)
225-237: The changes made to the
responseBuilderSimplefunction for additional validation ofmessage.eventare correctly implemented and align with the summary provided. The error messages are also more specific, which will aid in troubleshooting.250-250: The updated error message for unsupported event types is more informative and user-friendly, which aligns with the summary provided.
|
The issue is already fixed as part of #2846. But if there is anything we are doing extra, please feel free to ignore the mentioned PR with this one |
@sanpj2292 I added some validations in case |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/v0/destinations/sfmc/transform.js (5 hunks)
- src/v0/destinations/sfmc/transform.test.js (1 hunks)
Additional comments: 2
src/v0/destinations/sfmc/transform.js (1)
- 287-287: The addition of
responseBuilderSimpleto the module exports is consistent with the changes made in the file and the summary provided.src/v0/destinations/sfmc/transform.test.js (1)
- 1-178: The test suite for
responseBuilderSimpleis well-structured and covers a variety of scenarios including error handling and payload validation. The use of MockAxiosAdapter for simulating token retrieval is appropriate and the assertions within each test case are relevant to the expected outcomes. Good job on ensuring that the tests are comprehensive and that they validate both the success and error paths.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/v0/destinations/sfmc/transform.js (5 hunks)
- src/v0/destinations/sfmc/transform.test.js (1 hunks)
Additional comments: 7
src/v0/destinations/sfmc/transform.js (6)
3-8: The changes to the imports are consistent with the summary and the removal of
isEmptyfrom the imports is reflected in the code.225-226: The error message "Creating or updating contacts is disabled" has been updated and is now more specific, addressing the previous comment.
229-230: The error message "Event name is required for this track events" has been updated and is now more specific, addressing the previous comment.
232-237: The checks for
message.eventhave been added as per the summary, ensuring that the event name is not empty, is a string, and is present inhashMapExternalKey.251-252: The error message "Event type not supported" now includes the unsupported event type, addressing the previous comment.
288-288: The
responseBuilderSimplefunction is now explicitly exported, which is consistent with the summary.src/v0/destinations/sfmc/transform.test.js (1)
- 1-125: The test suite for
responseBuilderSimplefunction is comprehensive and covers various scenarios including identify calls, error handling for missing event names, unsupported event types, and mapped track calls. The setup of a mock Axios adapter is also correctly implemented for testing HTTP requests. The tests appear to be well-structured and should validate the functionality effectively.
|
Kudos, SonarCloud Quality Gate passed! |
Description of the change
> incident
For the above incident we are getting Generic Transformation error as the nature of the error is unhandled exception
The customer is wrongly instrumenting an invalid track event which does not containing
eventproperty for which we are missing validation. This PR Addresses the above issue with adding more validations for the SFMC destination for which the issue occursResolves INT-1085
Type of change
Related issues
Checklists
Development
Code review
Summary by CodeRabbit
New Features
Bug Fixes
Tests
responseBuilderSimplefunction to validate functionality and error handling.