Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an export viewer display bug where self-DM conversations could render as <external> due to the IM User field being left empty when both DM members are the current user.
Changes:
- Adjust DM-to-channel conversion to ensure self-DMs resolve a non-empty
Conversation.User. - Extend the
excepthelper with a fallback index parameter to handle the self-DM member list case. - Update/add unit tests for
mostFrequentMemberandexceptto cover the self-chat scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/structures/index.go | Updates DM user selection logic via except(..., 1) and extends except behavior/docs to avoid empty IM user IDs. |
| internal/structures/index_test.go | Renames/extends unit tests and adds cases covering self-chat detection and except fallback behavior. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
When "viewing" exports, the self-user chat was displayed as
<external>due toflawed logic of
exceptthat would fail to assign the "self" user to theconversation.user field.
Gory details
The dm chat entry in export's
dms.jsonwould have a slice of usersparticipating in DM, such as:
[ { "id": "DHYNUJ00Y", // conversation ID "created": 1555493778, "members": [ // participants "UHSD97ZA5", // this is you. "UABCDEFGH" // this is someone else. ] } ]To determine the user ID to assign to a DM conversation, the logic would:
and finding the "most frequent user" ID.
slice, that is NOT the user discovered on step 1.
For chats with others, it would correctly return the User ID of the "other"
participant. For example, for the payload in the beginning of this section, it
would return "UABCDEFGH"
The chat with "yourself" looks like this in the export's
dms.json:[ { "id": "DHYNUJ00Y", "created": 1555493778, "members": [ "UHSD97ZA5", "UHSD97ZA5" ] } ]UHSD97ZA5.As a result, the conversation would have an empty User ID and would not display
properly in the viewer.
Thank you, you're welcome, come again.