Skip to content

Conversation

@VladShturma
Copy link
Contributor

@VladShturma VladShturma commented May 14, 2025

Github Issue: #2013

CLA

  • I have signed the Stream CLA (required).
  • The code changes follow best practices
  • Code changes are tested (add some information if not applicable)

Description of the pull request

If you pin message, channel.state.pinnedMessages starts to store message twice. There already was a fix for such an issue, but for some reason it was overwritten later. I reapplied it and also added test

@codecov
Copy link

codecov bot commented May 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.21%. Comparing base (30c106e) to head (efbc7ba).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2252      +/-   ##
==========================================
+ Coverage   63.17%   63.21%   +0.03%     
==========================================
  Files         402      402              
  Lines       25229    25224       -5     
==========================================
+ Hits        15939    15945       +6     
+ Misses       9290     9279      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xsahil03x
Copy link
Member

Hey @VladShturma , Thank you for the PR.

Can you fix the formatting issues and add additional tests for cases like:

1: When a already pinned message is updated (should not add duplicate)
2: When a already pinned message is updated with pinned: false (should unpin the pinned message)

@xsahil03x xsahil03x requested a review from Copilot May 15, 2025 12:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses the duplicated pinned message issue by removing the redundant logic that added messages to pinnedMessages and adds a corresponding test to verify the fix.

  • Removed the code that manually appended pinned messages in ChannelClientState.
  • Added tests to ensure that a pinned message is only added once.
  • Updated the CHANGELOG to reflect the bug fix.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/stream_chat/test/src/client/channel_test.dart Added tests for the messageUpdated event to verify pinned behavior.
packages/stream_chat/lib/src/client/channel.dart Removed duplicate logic that led to inherent message duplication.
packages/stream_chat/CHANGELOG.md Updated the changelog with a note of the bug fix.

@VladShturma VladShturma force-pushed the fix/message-pin branch 2 times, most recently from 9a9f366 to 75768f7 Compare May 15, 2025 13:18
@VladShturma
Copy link
Contributor Author

Hello @xsahil03x
Done. I also added test to check that when not pinned message is updated with pinned: true it should not unpin another already pinned message.

Copy link
Member

@xsahil03x xsahil03x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @VladShturma LGTM, just added one comment

@VladShturma
Copy link
Contributor Author

Hey @xsahil03x done

@xsahil03x xsahil03x merged commit 6800319 into GetStream:master May 15, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants