Migrate sendReaction request body to generated ReactionRequest and SendReactionRequest models#6566
Open
gpunto wants to merge 2 commits into
Open
Migrate sendReaction request body to generated ReactionRequest and SendReactionRequest models#6566gpunto wants to merge 2 commits into
gpunto wants to merge 2 commits into
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
gpunto
force-pushed
the
migrate/send-reaction
branch
from
e688018 to
de6a5a7
Compare
…ndReactionRequest models
gpunto
force-pushed
the
migrate/send-reaction
branch
from
de6a5a7 to
21d230e
Compare
|
WalkthroughReaction sending now uses dedicated network request models. Reaction mapping and Moshi adapters support ChangesReaction request migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 (3)
stream-chat-android-client/src/main/java/io/getstream/chat/android/network/models/ReactionRequest.kt (1)
17-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the
@file:Suppressannotation or remove unnecessary suppressions.Per coding guidelines, suppressions should be avoided unless documented. The
@file:Suppresshere includes "ArrayInDataClass" and "EnumEntryName" which are not applicable to this file (no arrays, no enums). Consider adding a brief comment noting these are code-generation artifacts, or removing the inapplicable entries.🤖 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/network/models/ReactionRequest.kt` around lines 17 - 22, Update the file-level `@file`:Suppress annotation in ReactionRequest.kt by removing the inapplicable "ArrayInDataClass" and "EnumEntryName" entries, or document them with a brief comment if they are required code-generation artifacts; retain only necessary suppressions.Source: Coding guidelines
stream-chat-android-client/src/main/java/io/getstream/chat/android/network/models/SendReactionRequest.kt (1)
17-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument or remove unnecessary
@file:Suppressannotations.Per the project's coding guidelines, suppressions should be avoided unless documented. The suppressions
"ArrayInDataClass"and"EnumEntryName"are not applicable to this file (no arrays or enums present), and none of the suppressions have accompanying documentation explaining why they are needed.If this is generated from a template, consider adding a comment noting the code-generation origin, or remove the inapplicable suppressions for this file.
🤖 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/network/models/SendReactionRequest.kt` around lines 17 - 22, Remove the unnecessary `@file`:Suppress entries from SendReactionRequest, particularly "ArrayInDataClass" and "EnumEntryName"; retain only suppressions required by compiler warnings and document any retained generated-code suppressions with an explanatory comment.Source: Coding guidelines
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DtoMappingTest.kt (1)
241-256: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider explicit test coverage for both
emojiCodebranches.The test relies on
randomReaction()to determine whetheremojiCodeis null or non-null, so only one branch of thecustomlogic is exercised per run. Adding explicit test cases for both scenarios would improve coverage confidence.🧪 Suggested additional test cases
`@Test` fun `Reaction with null emojiCode maps custom to extraData only`() { val reaction = randomReaction().copy(emojiCode = null) val mapping = Fixture().get() val dto = with(mapping) { reaction.toDto() } val expected = ReactionRequest( type = reaction.type, createdAt = reaction.createdAt, score = reaction.score, updatedAt = reaction.updatedAt, custom = reaction.extraData, ) dto shouldBeEqualTo expected } `@Test` fun `Reaction with non-null emojiCode maps custom to extraData plus emoji_code`() { val reaction = randomReaction().copy(emojiCode = "smile") val mapping = Fixture().get() val dto = with(mapping) { reaction.toDto() } val expected = ReactionRequest( type = reaction.type, createdAt = reaction.createdAt, score = reaction.score, updatedAt = reaction.updatedAt, custom = reaction.extraData + ("emoji_code" to "smile"), ) dto shouldBeEqualTo expected }🤖 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/test/java/io/getstream/chat/android/client/api2/mapping/DtoMappingTest.kt` around lines 241 - 256, Expand the Reaction mapping tests to explicitly cover both emojiCode branches instead of relying on randomReaction(). In the Reaction is correctly mapped to Dto test or separate tests, create reactions with emojiCode set to null and to a non-null value, then assert custom equals extraData in the null case and extraData plus the emoji_code entry in the non-null case.
🤖 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/network/models/ReactionRequest.kt`:
- Around line 17-22: Update the file-level `@file`:Suppress annotation in
ReactionRequest.kt by removing the inapplicable "ArrayInDataClass" and
"EnumEntryName" entries, or document them with a brief comment if they are
required code-generation artifacts; retain only necessary suppressions.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/network/models/SendReactionRequest.kt`:
- Around line 17-22: Remove the unnecessary `@file`:Suppress entries from
SendReactionRequest, particularly "ArrayInDataClass" and "EnumEntryName"; retain
only suppressions required by compiler warnings and document any retained
generated-code suppressions with an explanatory comment.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DtoMappingTest.kt`:
- Around line 241-256: Expand the Reaction mapping tests to explicitly cover
both emojiCode branches instead of relying on randomReaction(). In the Reaction
is correctly mapped to Dto test or separate tests, create reactions with
emojiCode set to null and to a non-null value, then assert custom equals
extraData in the null case and extraData plus the emoji_code entry in the
non-null case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3ad4d2b6-a697-485c-8912-6936d2df5d1d
📒 Files selected for processing (11)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/MoshiChatApi.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/endpoint/MessageApi.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/ReactionDtos.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/requests/ReactionRequest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/adapters/CustomObjectDtoAdapter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/adapters/ReactionDtoAdapters.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/network/models/ReactionRequest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/network/models/SendReactionRequest.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/parser2/testdata/ReactionDtoTestData.kt
💤 Files with no reviewable changes (2)
- stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/ReactionDtos.kt
- stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/requests/ReactionRequest.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
Migrate the
sendReactionrequest body (POST /messages/{id}/reaction) to the generatedSendReactionRequest+ReactionRequestnetwork models. First patch-free generated model that carries a flattenedcustomfield. Part of the incremental OpenAPI model migration.Part of AND-1291
Implementation
internal SendReactionRequest(the{reaction, enforce_unique, skip_push}wrapper) andReactionRequest(the reaction object) innetwork.models. Remove the hand-writtenapi2.model.requests.ReactionRequestwrapper and the hand-writtenUpstreamReactionDto. Real names, no alias.CustomObjectDtoAdapter(backward-compatibly) with anextraDataPropertyNameparameter (defaults toextraData) and@Json-aware member-name resolution, so generated DTOs that hold the overflow map ascustomflatten correctly. Existing hand-written adapters are unchanged.UpstreamReactionDtoAdapternow targets the generatedReactionRequestwithextraDataPropertyName = "custom".Reaction.toDto()now emits the lean backend shape: it dropsmessage_id(comes from the URL path),user/user_id(set server-side), and foldsemoji_codeintocustom. Verified against the/chatSendReactionRequest/ReactionRequestGo structs.Testing
spotlessApply,apiCheck(no API drift),detekt,lint, and the full clienttestDebugUnitTestsuite pass.POST /messages/{id}/reactionsends{"reaction":{"type":"like","score":1,"migrate_probe":"v1","emoji_code":"👍"},"enforce_unique":false,"skip_push":false}(lean object, nomessage_id/user/user_id;customflattened to root), returns201, and the custom field +emoji_coderound-trip on the returned reaction.Summary by CodeRabbit