Skip to content

Commit

Permalink
queueMarkAsRead: Set timeout if there is mark request in less than 2s.
Browse files Browse the repository at this point in the history
If there are multiple requests to mark messages as read with interval
less than 2s, and if there is no further request then later were
stuck in the queue. Becuase there was only one caller to
`messagesFlags`, which was only called by user scroll event.
So even after reaching at the end of message list, unread banner is
visible with some unread count.

So now set timeout to send read message flag to server.

Fixes: zulip#3509
  • Loading branch information
jainkuniya committed Aug 21, 2019
1 parent 529f982 commit 8e89c18
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
21 changes: 20 additions & 1 deletion src/api/__tests__/queueMarkAsRead-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,22 @@ import { eg } from '../../__tests__/exampleData';
// $FlowFixMe Make flow understand about mocking
messagesFlags.default = jest.fn(() => {});

jest.useFakeTimers();

describe('queueMarkAsRead', () => {
beforeEach(() => {
resetAll();
jest.clearAllMocks();
jest.clearAllTimers();
});

test('should not call messagesFlags on consecutive calls of queueMarkAsRead', () => {
test('should not call messagesFlags on consecutive calls of queueMarkAsRead, setTimout on further calls', () => {
queueMarkAsRead(eg.selfAuth, [1, 2, 3]);
queueMarkAsRead(eg.selfAuth, [4, 5, 6]);
queueMarkAsRead(eg.selfAuth, [7, 8, 9]);
queueMarkAsRead(eg.selfAuth, [10, 11, 12]);

expect(setTimeout).toHaveBeenCalledTimes(1);
expect(messagesFlags.default).toHaveBeenCalledTimes(1);
});

Expand All @@ -37,4 +40,20 @@ describe('queueMarkAsRead', () => {

expect(messagesFlags.default).toHaveBeenCalledTimes(2);
});

test('should call messagesFlags after 2s to clear queue', async () => {
const currentDate = Date.now();
const secondCall = currentDate + 2100;
// $FlowFixMe Make flow understand about mocking
Date.now = jest.fn().mockReturnValue(currentDate);

queueMarkAsRead(eg.selfAuth, [1, 2, 3]);
queueMarkAsRead(eg.selfAuth, [4, 5, 6]);

// $FlowFixMe Make flow understand about mocking
Date.now = jest.fn().mockReturnValue(secondCall);
jest.runOnlyPendingTimers();

expect(messagesFlags.default).toHaveBeenCalledTimes(2);
});
});
12 changes: 10 additions & 2 deletions src/api/queueMarkAsRead.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
import type { Auth } from './transportTypes';
import messagesFlags from './messages/messagesFlags';

const TIME_INTERVAL_BETWEEN_CONSECUTIVE_CALLS_MS = 2000;
let unsentMessageIds = [];
let lastSentTime = 0;
let timeout = null;

/**
* Exported so that it can be used in test
Expand All @@ -12,13 +14,19 @@ let lastSentTime = 0;
export const resetAll = () => {
unsentMessageIds = [];
lastSentTime = 0;
timeout = null;
};

const processQueue = (auth: Auth) => {
if (Date.now() - lastSentTime > 2000) {
export const processQueue = (auth: Auth) => {
if (Date.now() - lastSentTime > TIME_INTERVAL_BETWEEN_CONSECUTIVE_CALLS_MS) {
messagesFlags(auth, unsentMessageIds, 'add', 'read');
unsentMessageIds = [];
lastSentTime = Date.now();
} else if (timeout === null) {
timeout = setTimeout(() => {
timeout = null;
processQueue(auth);
}, TIME_INTERVAL_BETWEEN_CONSECUTIVE_CALLS_MS);
}
};

Expand Down

0 comments on commit 8e89c18

Please sign in to comment.