Skip to content

Conversation

@xsahil03x
Copy link
Member

@xsahil03x xsahil03x commented Nov 25, 2025

Submit a pull request

Fixes: #2447

Description of the pull request

This PR ensures that the user's name in ConnectUserDetails is sourced from user.extraData['name'] instead of defaulting to user.id.

This prevents the user's ID from being inadvertently used as their display name during the WebSocket connection process.

Summary by CodeRabbit

  • Bug Fixes
    • Display names during WebSocket connection now come from users' profile data so identifiers no longer appear as names.
    • Attachment thumbnails now consider additional image sources (including packaged assets) so images load and resize more reliably.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Changed user-name sourcing for WebSocket connect to use user.extraData['name'] (via safeCast) and updated changelog; unified thumbnail image selection to prefer thumbUrl, imageUrl, then assetUrl through a single imageUrl variable for remote loading/resizing.

Changes

Cohort / File(s) Summary
Changelog
packages/stream_chat/CHANGELOG.md
Added an "Upcoming" section with a Fixed entry describing the fix for issue #2447 where a user's ID could be used as their display name during WebSocket connect.
WebSocket connection user details
packages/stream_chat/lib/src/ws/connect_user_details.dart
Added import for extension utilities and changed ConnectUserDetails.fromOwnUser() to set name from user.extraData['name'].safeCast<String>() instead of user.name (inline comment added).
Image attachment thumbnail
packages/stream_chat_flutter/lib/src/attachment/thumbnail/image_attachment_thumbnail.dart
Consolidated thumbnail source into a single imageUrl variable that falls back through thumbUrl, imageUrl, then assetUrl, and updated subsequent remote-loading/resizing logic to use this unified imageUrl.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ConnectUserDetails
  participant WebSocket
  Note over ConnectUserDetails: Name selection logic (changed)
  Client->>ConnectUserDetails: build from OwnUser
  ConnectUserDetails-->ConnectUserDetails: name = extraData['name'].safeCast<String>()\n(or null)
  ConnectUserDetails->>WebSocket: connect(payload with name)
  WebSocket->>Client: connection established
Loading
sequenceDiagram
  participant ThumbnailWidget
  participant RemoteLoader
  Note over ThumbnailWidget: imageUrl = thumbUrl ?? imageUrl ?? assetUrl (new unified path)
  ThumbnailWidget->>ThumbnailWidget: compute imageUrl
  alt imageUrl != null
    ThumbnailWidget->>RemoteLoader: request resized remote image
    RemoteLoader-->ThumbnailWidget: image bytes
  else
    ThumbnailWidget-->ThumbnailWidget: render local asset or placeholder
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review connect_user_details.dart for safeCast null handling and compatibility with existing OwnUser expectations.
  • Verify thumbnail logic covers assetUrl usage correctly and doesn't break local-asset rendering or resizing flows.
  • Confirm changelog format/conventions.

Possibly related PRs

Suggested reviewers

  • renefloor
  • Brazol

Poem

🐰 I hopped through extraData, nose alight,
Found names where shadows hid from sight.
Thumbnails pick their best path now, too —
Thumb, image, asset — a clever view.
Hooray for tidy code and carrots new! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: using extraData for name in ConnectUserDetails to fix the issue where user ID was used as display name.
Linked Issues check ✅ Passed The PR implementation directly addresses issue #2447 by changing ConnectUserDetails.fromOwnUser to derive name from user.extraData['name'] instead of user.name, preventing user ID from being used as display name.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the identified issue: import addition, name field derivation change, and CHANGELOG update are all directly related to the PR objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f6edfef and f74c1db.

📒 Files selected for processing (2)
  • packages/stream_chat/lib/src/ws/connect_user_details.dart (2 hunks)
  • packages/stream_chat_flutter/lib/src/attachment/thumbnail/image_attachment_thumbnail.dart (1 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1d0ba89 and 7cff64e.

📒 Files selected for processing (2)
  • packages/stream_chat/CHANGELOG.md (1 hunks)
  • packages/stream_chat/lib/src/ws/connect_user_details.dart (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: stream_chat_flutter
  • GitHub Check: stream_chat_localizations
  • GitHub Check: stream_chat_flutter_core
  • GitHub Check: stream_chat_persistence
  • GitHub Check: build (ios)
  • GitHub Check: test
  • GitHub Check: analyze
  • GitHub Check: analyze_legacy_versions
🔇 Additional comments (2)
packages/stream_chat/CHANGELOG.md (1)

1-6: LGTM!

The CHANGELOG entry accurately documents the fix and properly references issue #2447. The description clearly explains what was fixed.

packages/stream_chat/lib/src/ws/connect_user_details.dart (1)

39-40: Synchronization between user.name and user.extraData['name'] is properly maintained.

Verification confirms the User and OwnUser models correctly synchronize these fields:

  • The User constructor automatically sets name in extraData with key 'name' when provided
  • The name getter reads from extraData['name'] and falls back to id only if the value is null or empty
  • The copyWith method properly preserves name synchronization
  • OwnUser's copyWith uses the same synchronization pattern
  • OwnUser's merge method explicitly uses extraData['name']

The fix correctly uses user.extraData['name'].safeCast() instead of user.name to avoid the fallback to user ID, ensuring null is sent when no name is set.

This change ensures that the user's `name` in `ConnectUserDetails` is sourced from `user.extraData['name']` instead of defaulting to `user.id`.

This prevents the user's ID from being inadvertently used as their display name during the WebSocket connection process.
@xsahil03x xsahil03x force-pushed the fix/connect-user-details-name branch from 7cff64e to f6edfef Compare November 25, 2025 11:44
@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.60%. Comparing base (d8556e2) to head (f74c1db).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2451   +/-   ##
=======================================
  Coverage   64.60%   64.60%           
=======================================
  Files         420      420           
  Lines       26210    26210           
=======================================
+ Hits        16932    16934    +2     
+ Misses       9278     9276    -2     

☔ 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.

This change adds `assetUrl` as a fallback option when determining the image URL for a thumbnail.

The priority is now `thumbUrl`, then `imageUrl`, and finally `assetUrl`. This ensures that an image thumbnail can still be displayed even if the `thumbUrl` and `imageUrl` are not available, by using the local asset URL as a last resort.
@xsahil03x xsahil03x merged commit 279c6d2 into master Nov 25, 2025
12 of 14 checks passed
@xsahil03x xsahil03x deleted the fix/connect-user-details-name branch November 25, 2025 12:13
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.

Not specifying a username in the userobject for ConnectUser overwrites existing username

3 participants