-
Notifications
You must be signed in to change notification settings - Fork 368
fix(llc): fixed skipPush and skipEnrichUrl not preserving during message send or update retry #2330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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 WalkthroughThis change updates the message retry logic in the stream chat package to ensure that the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Channel
participant MessageState
participant Client
User->>Channel: retryMessage(message)
Channel->>MessageState: inspect failed state (get skipPush, skipEnrichUrl, partial update data)
alt sendingFailed
Channel->>Client: sendMessage(message, skipPush, skipEnrichUrl)
else updatingFailed
Channel->>Client: updateMessage(message, skipPush, skipEnrichUrl)
else partialUpdatingFailed
Channel->>Client: partialUpdateMessage(message, set, unset, skipEnrichUrl)
else deletingFailed
Channel->>Client: deleteMessage(message, hard)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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 (
|
67a7367
to
513ebd4
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v10.0.0 #2330 +/- ##
==========================================
Coverage ? 64.91%
==========================================
Files ? 417
Lines ? 25777
Branches ? 0
==========================================
Hits ? 16732
Misses ? 9045
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
513ebd4
to
d5d8b81
Compare
…age send or update retry
d5d8b81
to
54de13c
Compare
Hey @VladShturma , thanks for the fix. I will soon test this PR |
Hey @xsahil03x ! Friendly reminder about my PR |
Hey @VladShturma , The PR looks good to me but this is going to be a breaking change as we are introducing new fields in the I also have some fixes in mind around partialUpdate retry as we are not handling it correctly. (We are calling |
The `skipPush` parameter was missing from the `FailedState.updatingFailed` constructor and its associated logic. This commit adds the parameter and updates the relevant code to use it. This change ensures that the `skipPush` option is correctly propagated when a message update fails.
This commit introduces a new state, `partialUpdateFailed`, to the `MessageState` enum. This state is used when a partial update of a message fails. The `isUpdatingFailed` getter now also considers `partialUpdateFailed` as a failed update state. Tests have been updated to reflect these changes and ensure correct behavior when handling partial update failures, including retries and error handling.
# Conflicts: # packages/stream_chat/lib/src/core/models/message_state.freezed.dart # packages/stream_chat/lib/src/core/models/message_state.g.dart
This commit renames `partialUpdateFailed` and `PartialUpdateFailed` to `partialUpdatingFailed` and `PartialUpdatingFailed` respectively across the codebase. The `skipPush` parameter has also been added as a required parameter to `MessageState.updatingFailed` and its corresponding tests.
@xsahil03x yes, sounds good to me. Is there something that I should do? |
@VladShturma Nope, I can take it from here 👍🏼 |
# Conflicts: # packages/stream_chat/CHANGELOG.md # packages/stream_chat_flutter/test/src/message_actions_modal/message_actions_modal_test.dart
CLA
Description of the pull request
skipPush
andskipEnrichUrl
parameters are not preserved during message send or update retryHow to reproduce:
sendMessage
withskipPush
andskipEnrichUrl
set to trueActual result: push notification is sent for this message. Url encoding is not skipped
Expected result: push notification is not sent for this message. Url encoding is skipped
Summary by CodeRabbit
Bug Fixes
skipPush
andskipEnrichUrl
) are now correctly preserved during message send, update, and partial update retries, ensuring consistent retry behavior.Tests