Skip to content

Commit

Permalink
[lib] Use UPDATE_THREAD_READ_STATUS update to update the read status
Browse files Browse the repository at this point in the history
Summary:
This update is more convenient because it doesn't override the whole thread, which fixes a bug where the unread status operation sometimes swallows other operations. For the thick threads, this update should also bump a timestamp.

https://linear.app/comm/issue/ENG-9343/updating-thread-unread-status-sometimes-swallows-queued-operations

Test Plan:
1. Send a message as one user and check if a thread of the other used got marked as unread
2. Send a message as a user and check if a thread doesn't get marked as unread on another device of the same user
3. Check if marking as unread results in a thread being marked as unread on another device of the same user

Reviewers: kamil, will

Reviewed By: kamil

Subscribers: ashoat

Differential Revision: https://phab.comm.dev/D13424
  • Loading branch information
palys-swm committed Sep 23, 2024
1 parent 29f65d7 commit 359d803
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 48 deletions.
18 changes: 3 additions & 15 deletions lib/shared/dm-ops/change-thread-read-status-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,11 @@ const changeThreadReadStatusSpec: DMOperationSpec<DMChangeThreadReadStatusOperat

const updateInfos = [
{
type: updateTypes.UPDATE_THREAD,
type: updateTypes.UPDATE_THREAD_READ_STATUS,
id: uuid.v4(),
time,
threadInfo: {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
unread,
},
timestamps: {
...threadInfo.timestamps,
currentUser: {
...threadInfo.timestamps.currentUser,
unread: time,
},
},
},
threadID: threadInfo.id,
unread,
},
];
return {
Expand Down
35 changes: 3 additions & 32 deletions lib/shared/dm-ops/process-dm-ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,41 +288,12 @@ function useProcessDMOperation(): (
// We aren't checking if the unread timestamp is lower than the time.
// We're doing this because we want to flip the thread to unread after
// any new message from a non-viewer.
const updatedThreadInfo = threadInfo.minimallyEncoded
? {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
unread: true,
},
timestamps: {
...threadInfo.timestamps,
currentUser: {
...threadInfo.timestamps.currentUser,
unread: time,
},
},
}
: {
...threadInfo,
currentUser: {
...threadInfo.currentUser,
unread: true,
},
timestamps: {
...threadInfo.timestamps,
currentUser: {
...threadInfo.timestamps.currentUser,
unread: time,
},
},
};

updateInfos.push({
type: updateTypes.UPDATE_THREAD,
type: updateTypes.UPDATE_THREAD_READ_STATUS,
id: uuid.v4(),
time,
threadInfo: updatedThreadInfo,
threadID: threadInfo.id,
unread: true,
});
}

Expand Down
17 changes: 16 additions & 1 deletion lib/shared/updates/update-thread-read-status-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,22 @@ export const updateThreadReadStatusSpec: UpdateSpec<
const storeThreadInfo = storeThreadInfos[update.threadID];

let updatedThread;
if (storeThreadInfo.minimallyEncoded) {
if (storeThreadInfo.thick) {
updatedThread = {
...storeThreadInfo,
currentUser: {
...storeThreadInfo.currentUser,
unread: update.unread,
},
timestamps: {
...storeThreadInfo.timestamps,
currentUser: {
...storeThreadInfo.timestamps.currentUser,
unread: update.time,
},
},
};
} else if (storeThreadInfo.minimallyEncoded) {
updatedThread = {
...storeThreadInfo,
currentUser: {
Expand Down

0 comments on commit 359d803

Please sign in to comment.