Skip to content

Conversation

@jbsession
Copy link
Collaborator

SES-5135 : Chat with unknown group member - Misplaced control messages

Updates how we get threadId when creating control message.

Default threadId for Conversation screen set to 0F for now to avoid seeing weird messages with threadId = -1F.

@jbsession jbsession self-assigned this Jan 21, 2026
*/
@Deprecated("Use threadIdFlow instead")
val threadId: Long get() = threadIdFlow.value ?: -1L
val threadId: Long get() = threadIdFlow.value ?: 0L
Copy link
Collaborator

Choose a reason for hiding this comment

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

The issue here is that we are dealing with old code decisions regarding the threadId.
The reality is that we should handle null threadId instead of set values.
0L might be a valid thread id if the db starts its index with 0.
In an ideal world we would get rid of threadId entirely and rely on addresses but that's a bigger change...
For now maybe the best idea would be to deal with nullability instead of trying to fit 0 or -1
This would mean changing the logic here, but also removing the use of getThreadIdIfExistsFor to instead use getThreadId which is set with nullability in mind.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To add to Thomas' comment, another thing we can do to prevent the threadId == -1 messages from showing up, is to safeguard it in the messaging querying: make sure if threadId is -1 we return empty result

public Cursor getConversation(long threadId, boolean reverse, long offset, long limit) {
String order = MmsSmsColumns.NORMALIZED_DATE_SENT + (reverse ? " DESC" : " ASC");
String selection = MmsSmsColumns.THREAD_ID + " = " + threadId;
String selection = MmsSmsColumns.THREAD_ID + " = " + threadId + " AND " + THREAD_ID + " != " + -1L;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know that we need to change the sql queries here. Since you are passing the threadId in the constructor, should you simply check for -1 in the function and return early?
Same for the unread count below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ThomasSession I thought about this yesterday and it seems like in this unfortunate instance, you can't return early, the Cursor is a curse: it not only contains rows, it also contains information about columns so it would potentially break any downstream code if we just return early without filling in column names. So it should be fine for this instance to go through sql

@jbsession jbsession merged commit 712b2f5 into session-foundation:dev Jan 28, 2026
4 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.

3 participants