Conversation
Inaction and NestedObject support
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds nested-property support for profiles and events across the JS example app and native Android bridge, updates date-conversion to recurse into nested structures, and bumps package and native SDK versions for release 3.9.0. Changes
Sequence Diagram(s)mermaid ExampleApp->>AppUtils: Trigger nested-property action Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/index.js (1)
253-262:⚠️ Potential issue | 🟡 MinorDate objects directly inside arrays won't be converted to epoch.
The array branch recurses into object elements via
convertDateToEpochInProperties(item), but that function works on key-value entries of an object. If an array directly containsDateinstances (e.g.,[new Date(), new Date()]),Object.entries()on aDateyields nothing, so those dates won't be converted to the$D_epoch format.This is likely a rare edge case for CleverTap event properties, but worth noting. If direct
Datearray elements need conversion, the array iteration would need to check forDateinstances explicitly and replace them in-place.Proposed fix to also handle Date elements in arrays
} else if (Array.isArray(value)) { // Recursively convert dates in array elements - value.forEach(item => { - if (item !== null && typeof item === 'object') { + value.forEach((item, index) => { + if (Object.prototype.toString.call(item) === '[object Date]') { + value[index] = "$D_" + Math.floor(item.getTime() / 1000); + } else if (item !== null && typeof item === 'object') { convertDateToEpochInProperties(item); } });Example/app/App.js (1)
380-380:⚠️ Potential issue | 🟡 MinorPre-existing: stray comma produces an
undefinedentry in the array.Line 380 has a bare
,which inserts anundefinedelement into thesubCategoryarray under Product Config. This isn't introduced by this PR, but worth cleaning up to avoid unexpected rendering behavior inExpandableListView.Proposed fix
{ action: Actions.PRODUCT_CONFIG_GET_STRINGS, name: 'getStrings' }, { action: Actions.PRODUCT_CONFIG_RESET, name: 'reset' }, - , { action: Actions.PRODUCT_CONFIG_LAST_FETCH_TS,
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Inaction and NestedObject support
Summary by CodeRabbit
New Features
Updates