Skip to content

Commit

Permalink
Don't change left pane preview or order when someone leaves group
Browse files Browse the repository at this point in the history
  • Loading branch information
@scottnonnenberg-signal
scottnonnenberg-signal committed Jan 20, 2021
1 parent 1356625 commit be9721c
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 45 deletions.
10 changes: 10 additions & 0 deletions test/models/conversations_test.js
View file
Expand Up @@ -3,6 +3,9 @@

describe('Conversations', () => {
it('updates lastMessage even in race conditions with db', async () => {
const ourNumber = '+15550000000';
const ourUuid = window.getGuid();

// Creating a fake conversation
const conversation = new window.Whisper.Conversation({
id: '8c45efca-67a4-4026-b990-9537d5d1a08f',
Expand All @@ -12,6 +15,13 @@ describe('Conversations', () => {
});

const destinationE164 = '+15557654321';
window.textsecure.storage.user.setNumberAndDeviceId(
ourNumber,
2,
'my device'
);
window.textsecure.storage.user.setUuidAndDeviceId(ourUuid, 2);
window.ConversationController._initialFetchComplete = true;

// Creating a fake message
const now = Date.now();
Expand Down
14 changes: 12 additions & 2 deletions ts/models/conversations.ts
View file
Expand Up @@ -3089,11 +3089,21 @@ export class ConversationModel extends window.Backbone.Model<
return;
}

const ourConversationId = window.ConversationController.getOurConversationId();
if (!ourConversationId) {
throw new Error('updateLastMessage: Failed to fetch ourConversationId');
}

const conversationId = this.id;
let [previewMessage, activityMessage] = await Promise.all([
window.Signal.Data.getLastConversationPreview(this.id, {
window.Signal.Data.getLastConversationPreview({
conversationId,
ourConversationId,
Message: window.Whisper.Message,
}),
window.Signal.Data.getLastConversationActivity(this.id, {
window.Signal.Data.getLastConversationActivity({
conversationId,
ourConversationId,
Message: window.Whisper.Message,
}),
]);
Expand Down
42 changes: 26 additions & 16 deletions ts/sql/Client.ts
View file
Expand Up @@ -1031,27 +1031,37 @@ async function getNewerMessagesByConversation(

return new MessageCollection(handleMessageJSON(messages));
}
async function getLastConversationActivity(
conversationId: string,
options: {
Message: typeof MessageModel;
}
): Promise<MessageModel | undefined> {
const { Message } = options;
const result = await channels.getLastConversationActivity(conversationId);
async function getLastConversationActivity({
conversationId,
ourConversationId,
Message,
}: {
conversationId: string;
ourConversationId: string;
Message: typeof MessageModel;
}): Promise<MessageModel | undefined> {
const result = await channels.getLastConversationActivity({
conversationId,
ourConversationId,
});
if (result) {
return new Message(result);
}
return undefined;
}
async function getLastConversationPreview(
conversationId: string,
options: {
Message: typeof MessageModel;
}
): Promise<MessageModel | undefined> {
const { Message } = options;
const result = await channels.getLastConversationPreview(conversationId);
async function getLastConversationPreview({
conversationId,
ourConversationId,
Message,
}: {
conversationId: string;
ourConversationId: string;
Message: typeof MessageModel;
}): Promise<MessageModel | undefined> {
const result = await channels.getLastConversationPreview({
conversationId,
ourConversationId,
});
if (result) {
return new Message(result);
}
Expand Down
36 changes: 18 additions & 18 deletions ts/sql/Interface.ts
View file
Expand Up @@ -225,12 +225,14 @@ export type ServerInterface = DataInterface & {
conversationId: string,
options?: { limit?: number; receivedAt?: number; sentAt?: number }
) => Promise<Array<MessageTypeUnhydrated>>;
getLastConversationActivity: (
conversationId: string
) => Promise<MessageType | undefined>;
getLastConversationPreview: (
conversationId: string
) => Promise<MessageType | undefined>;
getLastConversationActivity: (options: {
conversationId: string;
ourConversationId: string;
}) => Promise<MessageType | undefined>;
getLastConversationPreview: (options: {
conversationId: string;
ourConversationId: string;
}) => Promise<MessageType | undefined>;
getNextExpiringMessage: () => Promise<MessageType>;
getNextTapToViewMessageToAgeOut: () => Promise<MessageType>;
getOutgoingWithoutExpiresAt: () => Promise<Array<MessageType>>;
Expand Down Expand Up @@ -323,18 +325,16 @@ export type ClientInterface = DataInterface & {
MessageCollection: typeof MessageModelCollectionType;
}
) => Promise<MessageModelCollectionType>;
getLastConversationActivity: (
conversationId: string,
options: {
Message: typeof MessageModel;
}
) => Promise<MessageModel | undefined>;
getLastConversationPreview: (
conversationId: string,
options: {
Message: typeof MessageModel;
}
) => Promise<MessageModel | undefined>;
getLastConversationActivity: (options: {
conversationId: string;
ourConversationId: string;
Message: typeof MessageModel;
}) => Promise<MessageModel | undefined>;
getLastConversationPreview: (options: {
conversationId: string;
ourConversationId: string;
Message: typeof MessageModel;
}) => Promise<MessageModel | undefined>;
getNextExpiringMessage: (options: {
Message: typeof MessageModel;
}) => Promise<MessageModel | null>;
Expand Down
64 changes: 55 additions & 9 deletions ts/sql/Server.ts
View file
Expand Up @@ -2795,19 +2795,44 @@ async function getNewestMessageForConversation(conversationId: string) {
return row;
}

async function getLastConversationActivity(
conversationId: string
): Promise<MessageType | null> {
async function getLastConversationActivity({
conversationId,
ourConversationId,
}: {
conversationId: string;
ourConversationId: string;
}): Promise<MessageType | null> {
const db = getInstance();
const row = await db.get(
`SELECT * FROM messages WHERE
conversationId = $conversationId AND
(type IS NULL OR type NOT IN ('profile-change', 'verified-change', 'message-history-unsynced', 'keychange', 'group-v1-migration')) AND
(json_extract(json, '$.expirationTimerUpdate.fromSync') IS NULL OR json_extract(json, '$.expirationTimerUpdate.fromSync') != 1)
(type IS NULL
OR
type NOT IN (
'profile-change',
'verified-change',
'message-history-unsynced',
'keychange',
'group-v1-migration'
)
) AND
(
json_extract(json, '$.expirationTimerUpdate.fromSync') IS NULL
OR
json_extract(json, '$.expirationTimerUpdate.fromSync') != 1
) AND NOT
(
type = 'group-v2-change' AND
json_extract(json, '$.groupV2Change.from') != $ourConversationId AND
json_extract(json, '$.groupV2Change.details.length') = 1 AND
json_extract(json, '$.groupV2Change.details[0].type') != 'member-remove' AND
json_extract(json, '$.groupV2Change.details[0].conversationId') != $ourConversationId
)
ORDER BY received_at DESC, sent_at DESC
LIMIT 1;`,
{
$conversationId: conversationId,
$ourConversationId: ourConversationId,
}
);

Expand All @@ -2817,18 +2842,39 @@ async function getLastConversationActivity(

return jsonToObject(row.json);
}
async function getLastConversationPreview(
conversationId: string
): Promise<MessageType | null> {
async function getLastConversationPreview({
conversationId,
ourConversationId,
}: {
conversationId: string;
ourConversationId: string;
}): Promise<MessageType | null> {
const db = getInstance();
const row = await db.get(
`SELECT * FROM messages WHERE
conversationId = $conversationId AND
(type IS NULL OR type NOT IN ('profile-change', 'verified-change', 'message-history-unsynced', 'group-v1-migration'))
(
type IS NULL
OR
type NOT IN (
'profile-change',
'verified-change',
'message-history-unsynced',
'group-v1-migration'
)
) AND NOT
(
type = 'group-v2-change' AND
json_extract(json, '$.groupV2Change.from') != $ourConversationId AND
json_extract(json, '$.groupV2Change.details.length') = 1 AND
json_extract(json, '$.groupV2Change.details[0].type') != 'member-remove' AND
json_extract(json, '$.groupV2Change.details[0].conversationId') != $ourConversationId
)
ORDER BY received_at DESC, sent_at DESC
LIMIT 1;`,
{
$conversationId: conversationId,
$ourConversationId: ourConversationId,
}
);

Expand Down

0 comments on commit be9721c

Please sign in to comment.