Page MenuHomePhabricator
Differential D14332

[keyserver] Consider all new messages in determineLatestMessagesPerThread
ClosedPublic
Actions

Authored by ashoat on Feb 11 2025, 12:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 26, 7:48 AM
Unknown Object (File)
Thu, Mar 20, 10:26 PM
Unknown Object (File)
Tue, Mar 11, 4:13 PM
Unknown Object (File)
Tue, Mar 11, 3:42 PM
Unknown Object (File)
Sat, Mar 1, 12:02 PM
Unknown Object (File)
Feb 25 2025, 4:51 PM
Unknown Object (File)
Feb 22 2025, 9:20 PM
Unknown Object (File)
Feb 22 2025, 9:20 PM
View All 31 Files
Subscribers
None

Details

Reviewers
tomek
angelika
Commits
rCOMMb5ea24eb7798: [keyserver] Consider all new messages in determineLatestMessagesPerThread
Summary

determineLatestMessagesPerThread currently has a subtle bug due to the fact that it only considers the last message index. (It iterates through all of them, but only the last one matters because it overwrites the results from earlier ones.)

latestMessage is getting set correctly in all cases, but the bug is that latestReadMessage can be wrong in some cases. In particular, if the most recent message doesn't set latestReadMessage, but an earlier one does, then the earlier one will be ignored and latestReadMessage will not be set.

This is a pretty extreme edge case because it only applies in the case that we have messages from different authors in the createMessages call, which I don't believe happens at any current callsites. But it still makes sense to fix the bug, and the issue becomes more relevant in future diffs where we introduce code to set latestMessageForUnreadCheck as well.

Depends on D14331

Test Plan

Tested in combination with later diffs, to confirm that it's possible for some message types to not set a thread to unread

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
keyserver/
src/
creators/
18 lines

Diff 47027

View Options

keyserver/src/creators/message-creator.js

Show First 20 Lines • Show All 131 Lines • ▼ Show 20 Lines try {
]); ]);
} catch {} } catch {}
const currentUserInfo = response ? loggedOutUserInfo : null; const currentUserInfo = response ? loggedOutUserInfo : null;
return { currentUserInfo, preRequestUserState, keyserverIDs }; return { currentUserInfo, preRequestUserState, keyserverIDs };
}; };
type UseLogOutOptions = { type UseLogOutOptions = {
+logOutType?: 'legacy' | 'primary_device' | 'secondary_device', +logOutType?: 'legacy' | 'primary_device' | 'secondary_device',
+skipIdentityLogOut?: boolean,
}; };
function useLogOut( function useLogOut(
options: UseLogOutOptions = {}, options: UseLogOutOptions = {},
): (keyserverIDs?: $ReadOnlyArray<string>) => Promise<LogOutResult> { ): (keyserverIDs?: $ReadOnlyArray<string>) => Promise<LogOutResult> {
const client = React.useContext(IdentityClientContext); const client = React.useContext(IdentityClientContext);
const identityClient = client?.identityClient; const identityClient = client?.identityClient;
const preRequestUserState = usePreRequestUserState(); const preRequestUserState = usePreRequestUserState();
const callKeyserverLogOut = useKeyserverCall(keyserverLogOut); const callKeyserverLogOut = useKeyserverCall(keyserverLogOut);
const commServicesAccessToken = useSelector( const commServicesAccessToken = useSelector(
state => state.commServicesAccessToken, state => state.commServicesAccessToken,
); );
const { logOutType } = options; const { logOutType, skipIdentityLogOut } = options;
return React.useCallback( return React.useCallback(
async (keyserverIDs?: $ReadOnlyArray<string>) => { async (keyserverIDs?: $ReadOnlyArray<string>) => {
const identityPromise = (async () => { const identityPromise = (async () => {
if (!usingCommServicesAccessToken || !commServicesAccessToken) { if (
skipIdentityLogOut ||
!usingCommServicesAccessToken ||
!commServicesAccessToken
) {
return; return;
} }
if (!identityClient) { if (!identityClient) {
throw new Error('Identity service client is not initialized'); throw new Error('Identity service client is not initialized');
} }
let callIdentityClientLogOut; let callIdentityClientLogOut;
if (logOutType === 'primary_device') { if (logOutType === 'primary_device') {
if (!identityClient.logOutPrimaryDevice) { if (!identityClient.logOutPrimaryDevice) {
Show All 36 Lines async (keyserverIDs?: $ReadOnlyArray<string>) => {
}; };
}, },
[ [
callKeyserverLogOut, callKeyserverLogOut,
commServicesAccessToken, commServicesAccessToken,
identityClient, identityClient,
logOutType, logOutType,
preRequestUserState, preRequestUserState,
skipIdentityLogOut,
], ],
); );
} }
function useIdentityLogOut(): () => Promise<LogOutResult> { function useIdentityLogOut(): () => Promise<LogOutResult> {
const client = React.useContext(IdentityClientContext); const client = React.useContext(IdentityClientContext);
const identityClient = client?.identityClient; const identityClient = client?.identityClient;
▲ Show 20 Lines • Show All 1,230 Lines • Show Last 20 Lines