fix: types and tests #17
Conversation
WalkthroughThe changes update test logic to match action classes by name instead of ID, remove unused imports, and refactor deserialization in the Changes
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves the Formbricks Android SDK by updating type usage and tests for better consistency and maintainability. Key changes include:
- Replacing the use of the "id" field with "name" in ActionClassReference and updating corresponding test comparisons.
- Introducing a custom Gson deserializer (SegmentFilterResourceDeserializer) to handle varied JSON formats for SegmentFilterResource.
- Centralizing Gson configuration in SurveyManager and performing cleanup in tests and configuration files.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sonar-project.properties | Added exclusion for Network/FormbricksService.kt to SonarQube checks. |
| SegmentFilterResourceDeserializer.kt | Added a custom deserializer to support both array and object JSON formats. |
| ActionClassReference.kt | Removed the "id" property and switched to using "name" for actions. |
| SurveyManager.kt | Centralized Gson instantiation with custom deserializer registration. |
| SurveyManagerInstrumentedTest.kt | Removed unused imports to improve readability. |
| FormbricksInstrumentedTest.kt | Updated test logic to compare action class references by "name" instead of "id". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
android/src/main/java/com/formbricks/android/manager/SurveyManager.kt (1)
85-85: Consider using the same Gson instance for serializationWhile using the default Gson instance for serialization will work, it would be more consistent to use the same custom Gson instance for both serialization and deserialization.
- environmentDataHolderJson = Gson().toJson(value) + environmentDataHolderJson = gson.toJson(value)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
android/src/androidTest/java/com/formbricks/android/FormbricksInstrumentedTest.kt(1 hunks)android/src/androidTest/java/com/formbricks/android/manager/SurveyManagerInstrumentedTest.kt(0 hunks)android/src/main/java/com/formbricks/android/manager/SurveyManager.kt(3 hunks)android/src/main/java/com/formbricks/android/model/environment/ActionClassReference.kt(0 hunks)android/src/main/java/com/formbricks/android/model/environment/SegmentFilterResourceDeserializer.kt(1 hunks)sonar-project.properties(1 hunks)
💤 Files with no reviewable changes (2)
- android/src/androidTest/java/com/formbricks/android/manager/SurveyManagerInstrumentedTest.kt
- android/src/main/java/com/formbricks/android/model/environment/ActionClassReference.kt
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: SonarCloud
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (6)
android/src/main/java/com/formbricks/android/model/environment/SegmentFilterResourceDeserializer.kt (2)
7-25: Well-structured polymorphic deserializer implementationThis deserialization approach elegantly handles the polymorphic nature of
SegmentFilterResourceby determining the appropriate type based on the JSON structure. The conditional logic clearly distinguishes between array (Group) and object (Primitive) cases.
15-22:Details
✅ Verification successful
Verify TypeToken usage
The TypeToken approach works well for deserializing generic collections, but ensure that the class hierarchy of
SegmentFilterResource,SegmentFilter, andSegmentPrimitiveFilterproperly supports this deserialization pattern.
🏁 Script executed:
#!/bin/bash # Look for the SegmentFilterResource class definition to confirm the class hierarchy fd -t f --exec grep -A 20 "sealed class SegmentFilterResource" {} \;Length of output: 1107
🏁 Script executed:
#!/bin/bash # Search for all uses of Gson’s TypeToken to locate the deserializer and see its context rg -n "TypeToken" -C 5Length of output: 2884
🏁 Script executed:
#!/bin/bash # Search for the definition of SegmentFilter in the codebase rg -n "sealed class SegmentFilter" -C 5 rg -n "interface SegmentFilter" -C 5 rg -n "class SegmentFilter" -C 5Length of output: 11209
TypeToken usage verified
GSON’s
TypeToken<List<SegmentFilter>>correctly resolves the genericList<SegmentFilter>, and sinceSegmentFilterandSegmentPrimitiveFilterare concrete data classes in the same package, the customJsonDeserializerwraps them intoSegmentFilterResourceas intended—no further changes needed.android/src/androidTest/java/com/formbricks/android/FormbricksInstrumentedTest.kt (1)
128-128: Correctly updated action class matching from ID to nameThe code has been appropriately updated to use
nameinstead ofidfor matching action classes, which aligns with the removal of theidproperty from theActionClassReferencedata class.android/src/main/java/com/formbricks/android/manager/SurveyManager.kt (3)
10-12: Appropriate imports addedNew imports for
SegmentFilterResourceandSegmentFilterResourceDeserializersupport the centralized Gson instance implementation.
42-49: Good implementation of centralized Gson instanceCreating a single Gson instance with the custom deserializer registered is an efficient approach. This implementation properly handles the polymorphic nature of
SegmentFilterResource.
73-73: Now using custom Gson instance for deserializationThe code correctly uses the custom Gson instance with the registered deserializer when parsing the JSON string from shared preferences.
|
This pull request introduces several changes to improve the functionality and maintainability of the Formbricks Android SDK. The most important updates include replacing the use of
idwithnameinActionClassReference, adding a custom deserializer forSegmentFilterResource, and updating the Gson configuration inSurveyManager. Additionally, minor cleanup and configuration updates were made.Functional Improvements:
Switch from
idtonameforActionClassReference: UpdatedActionClassReferenceto usenameinstead ofid, and adjusted related logic inFormbricksInstrumentedTestto reflect this change. ([[1]](https://github.com/formbricks/android/pull/17/files#diff-17f9055fd2a7b5b3d60e0cfbdc2bb867b9dc7d837fedbcdbe016425973c92fe7L8),[[2]](https://github.com/formbricks/android/pull/17/files#diff-d8025fc041721bf9548c55167b39fa47fc3ba369cdd762cfc3727d5e98692e48L128-R128))Custom Gson deserializer for
SegmentFilterResource: AddedSegmentFilterResourceDeserializerto handle both array and object JSON formats forSegmentFilterResource. ([android/src/main/java/com/formbricks/android/model/environment/SegmentFilterResourceDeserializer.ktR1-R25](https://github.com/formbricks/android/pull/17/files#diff-2f2cce680af0452e17e17d7821df2443867e87ccedd708117869d35cef84fe27R1-R25))Gson configuration in
SurveyManager: Introduced a centralized Gson instance inSurveyManagerwith the custom deserializer registered, replacing the default Gson usage. ([[1]](https://github.com/formbricks/android/pull/17/files#diff-bde2d5dbfa308a53395144d54ecc66f60c7665cf2b93c72e33221ae025969f60R42-R51),[[2]](https://github.com/formbricks/android/pull/17/files#diff-bde2d5dbfa308a53395144d54ecc66f60c7665cf2b93c72e33221ae025969f60L60-R73))Cleanup and Configuration:
Removed unused imports: Cleaned up unused imports in
SurveyManagerInstrumentedTestto improve readability. ([android/src/androidTest/java/com/formbricks/android/manager/SurveyManagerInstrumentedTest.ktL4-L12](https://github.com/formbricks/android/pull/17/files#diff-cde270f86fb07a8e127a02282a62efa7c15d47729e3530fafd3edd6784d101aaL4-L12))Updated sonar exclusions: Added
Network/FormbricksService.ktto the list of files excluded from SonarQube coverage analysis. ([sonar-project.propertiesR28](https://github.com/formbricks/android/pull/17/files#diff-43ed9d31bea2a6d518d69836bcd1a8e6bd81bf4df96c4745792c220ca5aa549cR28))Summary by CodeRabbit