Wire enhanced-mention fields through Message#6479
Draft
gpunto wants to merge 1 commit into
Draft
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
|
DB Entities have been updated. Do we need to upgrade DB Version? |
Contributor
SDK Size Comparison 📏
|
gpunto
force-pushed
the
pr1-enhanced-mentions-data
branch
from
f4808c1 to
56e5d2b
Compare
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
WalkthroughThis PR adds comprehensive user group mention support throughout the Stream Chat Android SDK, enabling messages to track mentions of ChangesUser Group Mentions and Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/converter/internal/UserGroupConverter.kt (1)
32-35: 💤 Low valueConsider defensive fallback for JSON parse failures.
If
fromJsonencounters malformed JSON, it returnsnull. While Room will use the entity field's default (emptyList()), explicitly handling parse failure would be more defensive and clearer.♻️ Suggested improvement
`@TypeConverter` fun stringToUserGroupList(data: String?): List<UserGroup>? { if (data.isNullOrEmpty() || data == "null") return emptyList() - return listAdapter.fromJson(data) + return listAdapter.fromJson(data) ?: emptyList() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/converter/internal/UserGroupConverter.kt` around lines 32 - 35, The stringToUserGroupList function should defensively handle malformed JSON by guarding the call to listAdapter.fromJson; if data is null/empty return emptyList() as already done, and if listAdapter.fromJson throws or returns null, catch the exception or check for null and return emptyList() instead of propagating null—update UserGroupConverter.stringToUserGroupList to wrap the fromJson call in a try/catch (or null-check) and ensure it always returns a non-null List<UserGroup> fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/converter/internal/UserGroupConverter.kt`:
- Around line 32-35: The stringToUserGroupList function should defensively
handle malformed JSON by guarding the call to listAdapter.fromJson; if data is
null/empty return emptyList() as already done, and if listAdapter.fromJson
throws or returns null, catch the exception or check for null and return
emptyList() instead of propagating null—update
UserGroupConverter.stringToUserGroupList to wrap the fromJson call in a
try/catch (or null-check) and ensure it always returns a non-null
List<UserGroup> fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2d2004e5-7e7d-41ff-b6bf-6de0da8ce2a8
📒 Files selected for processing (23)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/DomainMapping.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/DtoMapping.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/ConfigDto.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/MessageDtos.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/UserGroupDtos.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/converter/internal/UserGroupConverter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/internal/ChatDatabase.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/channelconfig/internal/ChannelConfigEntity.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/channelconfig/internal/ChannelConfigMapper.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/message/internal/MessageEntity.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/message/internal/MessageMapper.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/message/internal/ReplyMessageEntity.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/Mother.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DtoMappingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/Mother.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/repository/domain/message/internal/MessageMapperTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/testdata/MessageDtoTestData.ktstream-chat-android-core/api/stream-chat-android-core.apistream-chat-android-core/src/main/java/io/getstream/chat/android/models/Config.ktstream-chat-android-core/src/main/java/io/getstream/chat/android/models/Message.ktstream-chat-android-core/src/main/java/io/getstream/chat/android/models/UserGroup.ktstream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Add the new enhanced-mention fields and
Config.pushLevelto the SDK's data layer so apps can read and write them end-to-end. UI lands in later PRs.This is PR 1 in the enhanced-mentions series.
Part of AND-1175
Implementation
MessagegetsmentionedHere,mentionedChannel,mentionedRoles: List<String>,mentionedGroups: List<UserGroup>.UserGroup/UserGroupMembermodels;Config.pushLevel: String?.mentioned_group_ids(id list); downstream receives hydratedmentioned_groups.DomainMapping/DtoMappinground-trip the new fields.MessageEntity/ReplyMessageEntityget a column per field;mentionedGroupsis persisted as JSON via a newUserGroupConverter.ChatDatabasebumps its schema version.Testing
MessageMapperTest,DtoMappingTest,DomainMappingTest,DownstreamMessageDtoAdapterTestround-trip the new fields.mentionedHere = trueand confirm it round-trips through Room.Summary by CodeRabbit
Release Notes
@here,@channel, user groups, and roles.