Skip to content

Commit 6327f64

Browse files
gnpricegnprice
committed
presence: Use server's offline threshold in getAggregatedPresence
1 parent 6739e87 commit 6327f64

File tree

2 files changed

+90
-37
lines changed

2 files changed

+90
-37
lines changed

‎src/presence/__tests__/presenceModel-test.js

Lines changed: 76 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import type { PerAccountState } from '../../reduxTypes';
1818

1919
const currentTimestamp = Date.now() / 1000;
2020

21+
// TODO test offlineThresholdSeconds varying
22+
const offlineThresholdSeconds = 140;
23+
2124
describe('getAggregatedPresence', () => {
2225
const oldAggregatedClientPresence = {
2326
client: 'website',
@@ -27,11 +30,18 @@ describe('getAggregatedPresence', () => {
2730

2831
test('aggregated status is active if any of the client has status active with age less than threshold', () => {
2932
expect(
30-
getAggregatedPresence({
31-
website: { client: 'website', timestamp: currentTimestamp - 100, status: 'idle' },
32-
zulipMobile: { client: 'zulipMobile', timestamp: currentTimestamp - 120, status: 'active' },
33-
aggregated: oldAggregatedClientPresence,
34-
}),
33+
getAggregatedPresence(
34+
{
35+
website: { client: 'website', timestamp: currentTimestamp - 100, status: 'idle' },
36+
zulipMobile: {
37+
client: 'zulipMobile',
38+
timestamp: currentTimestamp - 120,
39+
status: 'active',
40+
},
41+
aggregated: oldAggregatedClientPresence,
42+
},
43+
offlineThresholdSeconds,
44+
),
3545
).toEqual({
3646
client: 'zulipMobile',
3747
status: 'active',
@@ -41,49 +51,86 @@ describe('getAggregatedPresence', () => {
4151

4252
test('aggregated status is idle if any of the client has status idle with age less than threshold and no client has status active with age has than threshold', () => {
4353
expect(
44-
getAggregatedPresence({
45-
website: { client: 'website', timestamp: currentTimestamp - 100, status: 'idle' },
46-
zulipMobile: { client: 'zulipMobile', timestamp: currentTimestamp - 220, status: 'active' },
47-
aggregated: oldAggregatedClientPresence,
48-
}),
54+
getAggregatedPresence(
55+
{
56+
website: { client: 'website', timestamp: currentTimestamp - 100, status: 'idle' },
57+
zulipMobile: {
58+
client: 'zulipMobile',
59+
timestamp: currentTimestamp - 220,
60+
status: 'active',
61+
},
62+
aggregated: oldAggregatedClientPresence,
63+
},
64+
offlineThresholdSeconds,
65+
),
4966
).toEqual({ client: 'website', status: 'idle', timestamp: currentTimestamp - 100 });
5067
});
5168

5269
test('aggregated status is offline if no client has status active or idle with age less than threshold', () => {
5370
expect(
54-
getAggregatedPresence({
55-
zulipMobile: { client: 'zulipMobile', timestamp: currentTimestamp - 400, status: 'active' },
56-
website: { client: 'website', timestamp: currentTimestamp - 200, status: 'idle' },
57-
zulipAndroid: {
58-
client: 'zulipAndroid',
59-
timestamp: currentTimestamp - 500,
60-
status: 'active',
71+
getAggregatedPresence(
72+
{
73+
zulipMobile: {
74+
client: 'zulipMobile',
75+
timestamp: currentTimestamp - 400,
76+
status: 'active',
77+
},
78+
website: { client: 'website', timestamp: currentTimestamp - 200, status: 'idle' },
79+
zulipAndroid: {
80+
client: 'zulipAndroid',
81+
timestamp: currentTimestamp - 500,
82+
status: 'active',
83+
},
84+
aggregated: oldAggregatedClientPresence,
6185
},
62-
aggregated: oldAggregatedClientPresence,
63-
}),
86+
offlineThresholdSeconds,
87+
),
6488
).toEqual({ client: '', timestamp: currentTimestamp - 200, status: 'offline' });
6589
});
6690

67-
test('do not consider presence if its age is greater than OFFLINE_THRESHOLD_SECS', () => {
91+
test('do not consider presence if its age is greater than offline threshold', () => {
6892
expect(
69-
getAggregatedPresence({
70-
website: { client: 'website', timestamp: currentTimestamp - 300, status: 'active' },
71-
zulipMobile: { client: 'zulipMobile', timestamp: currentTimestamp - 10, status: 'idle' },
72-
aggregated: oldAggregatedClientPresence,
73-
}),
93+
getAggregatedPresence(
94+
{
95+
website: { client: 'website', timestamp: currentTimestamp - 300, status: 'active' },
96+
zulipMobile: { client: 'zulipMobile', timestamp: currentTimestamp - 10, status: 'idle' },
97+
aggregated: oldAggregatedClientPresence,
98+
},
99+
offlineThresholdSeconds,
100+
),
74101
).toEqual({
75102
client: 'zulipMobile',
76103
status: 'idle',
77104
timestamp: currentTimestamp - 10,
78105
});
79106
});
80107

108+
test('Use specified offline threshold', () => {
109+
expect(
110+
getAggregatedPresence(
111+
{
112+
website: { client: 'website', timestamp: currentTimestamp - 300, status: 'active' },
113+
zulipMobile: { client: 'zulipMobile', timestamp: currentTimestamp - 10, status: 'idle' },
114+
aggregated: oldAggregatedClientPresence,
115+
},
116+
360,
117+
),
118+
).toEqual({
119+
client: 'website',
120+
status: 'active',
121+
timestamp: currentTimestamp - 10,
122+
});
123+
});
124+
81125
test('Do not consider old aggregated', () => {
82126
expect(
83-
getAggregatedPresence({
84-
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 },
85-
website: { client: 'website', status: 'idle', timestamp: currentTimestamp - 10 },
86-
}),
127+
getAggregatedPresence(
128+
{
129+
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 },
130+
website: { client: 'website', status: 'idle', timestamp: currentTimestamp - 10 },
131+
},
132+
offlineThresholdSeconds,
133+
),
87134
).toEqual({ client: 'website', status: 'idle', timestamp: currentTimestamp - 10 });
88135
});
89136
});

‎src/presence/presenceModel.js

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,17 @@ const OFFLINE_THRESHOLD_SECS = 140;
6767
* This logic should match `status_from_timestamp` in the web app's
6868
* `static/js/presence.js` at 1ae07b93d^.
6969
*/
70-
export const getAggregatedPresence = (presence: UserPresence): ClientPresence => {
70+
export const getAggregatedPresence = (
71+
presence: UserPresence,
72+
offlineThresholdSeconds: number,
73+
): ClientPresence => {
7174
/* Gives an artificial ClientPresence object where
7275
* - `timestamp` is the latest `timestamp` of all ClientPresence
7376
* objects in the input, and
7477
* - `status` is the greatest `status` of all *recent* ClientPresence
7578
* objects, where `active` > `idle` > `offline`. If there are no
7679
* recent ClientPresence objects (i.e., they are all at least as
77-
* old as OFFLINE_THRESHOLD_SECS), `status` is 'offline'. If there
80+
* old as offlineThresholdSeconds), `status` is 'offline'. If there
7881
* are several ClientPresence objects with the greatest
7982
* PresenceStatus, an arbitrary one is chosen.
8083
* - `client` is the `client` of the ClientPresence object from the
@@ -101,7 +104,7 @@ export const getAggregatedPresence = (presence: UserPresence): ClientPresence =>
101104
if (timestamp < devicePresence.timestamp) {
102105
timestamp = devicePresence.timestamp;
103106
}
104-
if (age < OFFLINE_THRESHOLD_SECS) {
107+
if (age < offlineThresholdSeconds) {
105108
if (presenceStatusGeq(devicePresence.status, status)) {
106109
client = device;
107110
status = devicePresence.status;
@@ -241,11 +244,14 @@ export function reducer(
241244
({
242245
...userPresence,
243246
...action.presence,
244-
// $FlowIssue[cannot-spread-indexer] https://github.com/facebook/flow/issues/8276
245-
aggregated: getAggregatedPresence({
246-
...userPresence,
247-
...action.presence,
248-
}),
247+
aggregated: getAggregatedPresence(
248+
// $FlowIssue[cannot-spread-indexer] https://github.com/facebook/flow/issues/8276
249+
{
250+
...userPresence,
251+
...action.presence,
252+
},
253+
state.offlineThresholdSeconds,
254+
),
249255
}),
250256
),
251257
};

0 commit comments

Comments
 (0)