Skip to content

Commit

Permalink
Fix DM activity handler not to send unnecessary read status update
Browse files Browse the repository at this point in the history
Summary: This differential prevents DM activity handler from spamming unread status unpdates by sending updates only for threads that are unread when we enter them and using debouncing when receiving messages to currently active thread.

Test Plan:
1. Add console log to each portion of sent dm's
2. Ensure that when entering thick thread updates are sent once only if the thread was unread.
3. Ensuure that after receiving stream of messages to thick thread, unread updates are sent only once 5 seconds after the last message.

Reviewers: ashoat, tomek, kamil

Reviewed By: tomek

Differential Revision: https://phab.comm.dev/D13383
  • Loading branch information
marcinwasowicz committed Sep 20, 2024
1 parent cc116bc commit 7c26aa8
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 14 deletions.
64 changes: 51 additions & 13 deletions lib/handlers/dm-activity-handler.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow

import invariant from 'invariant';
import _debounce from 'lodash/debounce.js';
import * as React from 'react';

import {
Expand All @@ -11,26 +11,23 @@ import { useProcessAndSendDMOperation } from '../shared/dm-ops/process-dm-ops.js
import { getMostRecentNonLocalMessageID } from '../shared/message-utils.js';
import { threadIsPending } from '../shared/thread-utils.js';
import type { DMChangeThreadReadStatusOperation } from '../types/dm-ops.js';
import type { RawThreadInfo } from '../types/minimally-encoded-thread-permissions-types.js';
import { threadTypeIsThick } from '../types/thread-types-enum.js';
import { useDispatchActionPromise } from '../utils/redux-promise-utils.js';
import { useSelector } from '../utils/redux-utils.js';

const ACTIVITY_UPDATE_DURATION = 5000;

function useUpdateDMActivity(): (
viewerID: string,
activeThreadInfo: RawThreadInfo,
activeThread: string,
) => Promise<void> {
const processAndSendDMOperation = useProcessAndSendDMOperation();
return React.useCallback(
async (viewerID: string, activeThreadInfo: RawThreadInfo) => {
invariant(
threadTypeIsThick(activeThreadInfo.type),
'thread must be thick',
);
async (viewerID: string, activeThread: string) => {
const op: DMChangeThreadReadStatusOperation = {
type: 'change_thread_read_status',
time: Date.now(),
threadID: activeThreadInfo.id,
threadID: activeThread,
creatorID: viewerID,
unread: false,
};
Expand All @@ -47,6 +44,19 @@ function useUpdateDMActivity(): (
);
}

function getUpateActivityAfterLatestMessageChange(
viewerID: ?string,
activeThread: ?string,
updateDMActivity: (viewerID: string, activeThread: string) => Promise<void>,
) {
return _debounce(() => {
if (!activeThread || !viewerID) {
return;
}
void updateDMActivity(viewerID, activeThread);
}, ACTIVITY_UPDATE_DURATION);
}

function useDMActivityHandler(activeThread: ?string): void {
const activeThreadInfo = useSelector(state =>
activeThread ? state.threadStore.threadInfos[activeThread] : null,
Expand All @@ -67,6 +77,14 @@ function useDMActivityHandler(activeThread: ?string): void {
const updateDMActivity = useUpdateDMActivity();
const dispatchActionPromise = useDispatchActionPromise();

const updateActivityAfterLatestMessageChangeRef = React.useRef(
getUpateActivityAfterLatestMessageChange(
viewerID,
activeThread,
updateDMActivity,
),
);

React.useEffect(() => {
const prevActiveThread = prevActiveThreadRef.current;
const prevActiveThreadLatestMessage =
Expand All @@ -75,19 +93,39 @@ function useDMActivityHandler(activeThread: ?string): void {
prevActiveThreadRef.current = activeThread;
prevActiveThreadLatestMessageRef.current = activeThreadLatestMessage;

const activeThreadChanged = prevActiveThread !== activeThread;
if (activeThreadChanged) {
updateActivityAfterLatestMessageChangeRef.current =
getUpateActivityAfterLatestMessageChange(
viewerID,
activeThread,
updateDMActivity,
);
}

if (
!viewerID ||
!activeThread ||
!activeThreadInfo ||
!threadTypeIsThick(activeThreadInfo.type) ||
threadIsPending(activeThread) ||
(activeThread === prevActiveThread &&
activeThreadLatestMessage === prevActiveThreadLatestMessage)
threadIsPending(activeThread)
) {
return;
}

if (activeThreadInfo.currentUser.unread) {
void updateDMActivity(viewerID, activeThread);
return;
}

if (
activeThreadChanged ||
activeThreadLatestMessage === prevActiveThreadLatestMessage
) {
return;
}

void updateDMActivity(viewerID, activeThreadInfo);
updateActivityAfterLatestMessageChangeRef.current();
}, [
updateDMActivity,
dispatchActionPromise,
Expand Down
4 changes: 3 additions & 1 deletion web/redux/redux-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import type { StoreOperations } from 'lib/types/store-ops-types.js';
import type { SyncedMetadataStore } from 'lib/types/synced-metadata-types.js';
import type { GlobalThemeInfo } from 'lib/types/theme-types.js';
import type { ThreadActivityStore } from 'lib/types/thread-activity-types';
import { threadTypeIsThick } from 'lib/types/thread-types-enum.js';
import type { ThreadStore } from 'lib/types/thread-types.js';
import type { TunnelbrokerDeviceToken } from 'lib/types/tunnelbroker-device-token-types.js';
import type { CurrentUserInfo, UserStore } from 'lib/types/user-types.js';
Expand Down Expand Up @@ -455,7 +456,8 @@ function validateStateAndQueueOpsProcessing(
'hasFocus' in document &&
document.hasFocus() &&
!state.navInfo.pendingThread &&
state.threadStore.threadInfos[activeThread].currentUser.unread
state.threadStore.threadInfos[activeThread].currentUser.unread &&
!threadTypeIsThick(state.threadStore.threadInfos[activeThread].type)
) {
// Makes sure a currently focused thread is never unread
const activeThreadInfo = state.threadStore.threadInfos[activeThread];
Expand Down

0 comments on commit 7c26aa8

Please sign in to comment.