Skip to content

Commit e349ffa

Browse files
committed
presence: Encapsulate getUserLastActiveAsRelativeTimeString's various data needs
This will make it easier for this selector to add yet one more little piece of data it needs to consult. It'll also give us the opportunity to simplify the logic a bit; we'll do that next. While here, reduce the number of separate times we consult Date.now.
1 parent 32052e6 commit e349ffa

File tree

3 files changed

+66
-60
lines changed

3 files changed

+66
-60
lines changed

src/presence/__tests__/presenceModel-test.js

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,18 @@
22

33
import deepFreeze from 'deep-freeze';
44

5+
import Immutable from 'immutable';
56
import * as eg from '../../__tests__/lib/exampleData';
67
import { PRESENCE_RESPONSE, EVENT_PRESENCE } from '../../actionConstants';
78
import {
89
reducer as presenceReducer,
910
getAggregatedPresence,
10-
userLastActiveAsRelativeTimeString,
1111
statusFromPresence,
1212
statusFromPresenceAndUserStatus,
13+
getUserLastActiveAsRelativeTimeString,
1314
} from '../presenceModel';
1415
import { makePresenceState } from './presence-testlib';
16+
import type { UserPresence, UserStatus } from '../../api/modelTypes';
1517

1618
const currentTimestamp = Date.now() / 1000;
1719

@@ -85,52 +87,59 @@ describe('getAggregatedPresence', () => {
8587
});
8688
});
8789

88-
describe('userLastActiveAsRelativeTimeString', () => {
89-
test('given a presence return a relative time', () => {
90-
expect(
91-
userLastActiveAsRelativeTimeString(
92-
{
93-
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 240 },
94-
},
95-
{ away: false, status_text: null, status_emoji: null },
96-
eg.recentZulipFeatureLevel,
97-
),
98-
).toBe('4 minutes ago');
90+
describe('getUserLastActiveAsRelativeTimeString', () => {
91+
function lastActiveString(args: {
92+
userPresence: UserPresence,
93+
userStatus: UserStatus,
94+
zulipFeatureLevel?: number,
95+
}): string | null {
96+
const { userPresence, userStatus, zulipFeatureLevel = eg.recentZulipFeatureLevel } = args;
97+
return getUserLastActiveAsRelativeTimeString(
98+
eg.reduxStatePlus({
99+
presence: makePresenceState([[eg.otherUser, userPresence]]),
100+
userStatuses: Immutable.Map([[eg.otherUser.user_id, userStatus]]),
101+
accounts: [{ ...eg.plusReduxState.accounts[0], zulipFeatureLevel }],
102+
}),
103+
eg.otherUser,
104+
currentTimestamp * 1000,
105+
);
106+
}
107+
108+
test('a recent time', () => {
109+
const userPresence = {
110+
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 240 },
111+
};
112+
const userStatus = { away: false, status_text: null, status_emoji: null };
113+
expect(lastActiveString({ userPresence, userStatus })).toBe('4 minutes ago');
99114
});
100115

101116
test('if less than a threshold, the user is currently active', () => {
102-
expect(
103-
userLastActiveAsRelativeTimeString(
104-
{
105-
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 },
106-
},
107-
{ away: false, status_text: null, status_emoji: null },
108-
eg.recentZulipFeatureLevel,
109-
),
110-
).toBe('now');
117+
const userPresence = {
118+
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 },
119+
};
120+
const userStatus = { away: false, status_text: null, status_emoji: null };
121+
expect(lastActiveString({ userPresence, userStatus })).toBe('now');
111122
});
112123

113124
// TODO(server-6.0): Remove
114125
test('Pre-FL 148: if less than a day and user is "away", use imprecise "today"', () => {
115-
expect(
116-
userLastActiveAsRelativeTimeString(
117-
{ aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 } },
118-
{ away: true, status_text: null, status_emoji: null },
119-
147,
120-
),
121-
).toBe('today');
126+
const userPresence = {
127+
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 },
128+
};
129+
const userStatus = { away: true, status_text: null, status_emoji: null };
130+
const zulipFeatureLevel = 147;
131+
expect(lastActiveString({ userPresence, userStatus, zulipFeatureLevel })).toBe('today');
122132
});
123133

124134
// TODO(server-6.0): Remove, once this test case is redundant with those
125135
// above after the status parameter is gone.
126136
test('FL 148: if less than a day and user is "away", *don\'t* use imprecise "today"', () => {
127-
expect(
128-
userLastActiveAsRelativeTimeString(
129-
{ aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 } },
130-
{ away: true, status_text: null, status_emoji: null },
131-
148,
132-
),
133-
).not.toBe('today');
137+
const userPresence = {
138+
aggregated: { client: 'website', status: 'active', timestamp: currentTimestamp - 100 },
139+
};
140+
const userStatus = { away: true, status_text: null, status_emoji: null };
141+
const zulipFeatureLevel = 148;
142+
expect(lastActiveString({ userPresence, userStatus, zulipFeatureLevel })).not.toBe('today');
134143
});
135144
});
136145

src/presence/presenceModel.js

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
REGISTER_COMPLETE,
2626
RESET_ACCOUNT_DATA,
2727
} from '../actionConstants';
28+
import type { UserOrBot } from '../api/modelTypes';
2829

2930
//
3031
//
@@ -111,11 +112,18 @@ export const getAggregatedPresence = (presence: UserPresence): ClientPresence =>
111112
return { client, status, timestamp };
112113
};
113114

114-
export const userLastActiveAsRelativeTimeString = (
115-
presence: UserPresence,
116-
status: UserStatus,
117-
zulipFeatureLevel: number,
118-
): string => {
115+
export function getUserLastActiveAsRelativeTimeString(
116+
state: PerAccountState,
117+
user: UserOrBot,
118+
dateNow: number,
119+
): string | null {
120+
const presence = getUserPresenceByEmail(getPresence(state), user.email);
121+
if (!presence) {
122+
return null;
123+
}
124+
const userStatus = getUserStatus(state, user.user_id);
125+
const zulipFeatureLevel = getZulipFeatureLevel(state);
126+
119127
if (!presence || !presence.aggregated) {
120128
return 'never';
121129
}
@@ -124,17 +132,17 @@ export const userLastActiveAsRelativeTimeString = (
124132

125133
// "Invisible mode", new in FL 148, doesn't involve UserStatus['away']:
126134
// https://chat.zulip.org/#narrow/stream/2-general/topic/.22unavailable.22.20status/near/1454779
127-
// TODO(server-6.0): Remove this `if` block and the `status` parameter.
128-
if (zulipFeatureLevel < 148 && status.away && differenceInDays(Date.now(), lastTimeActive) < 1) {
135+
// TODO(server-6.0): Simplify this away.
136+
if (zulipFeatureLevel < 148 && userStatus.away && differenceInDays(dateNow, lastTimeActive) < 1) {
129137
// Be vague when an unavailable user is recently online.
130138
// TODO: This phrasing doesn't really match the logic and can be misleading.
131139
return 'today';
132140
}
133141

134-
return differenceInSeconds(Date.now(), lastTimeActive) < OFFLINE_THRESHOLD_SECS
142+
return differenceInSeconds(dateNow, lastTimeActive) < OFFLINE_THRESHOLD_SECS
135143
? 'now'
136144
: `${formatDistanceToNow(lastTimeActive)} ago`;
137-
};
145+
}
138146

139147
export const statusFromPresence = (presence: UserPresence | void): PresenceStatus => {
140148
if (!presence || !presence.aggregated) {

src/title/ActivityText.js

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,7 @@ import type { TextStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet
66

77
import type { UserOrBot } from '../types';
88
import { useSelector } from '../react-redux';
9-
import { getZulipFeatureLevel } from '../selectors';
10-
import {
11-
getPresence,
12-
getUserPresenceByEmail,
13-
userLastActiveAsRelativeTimeString,
14-
} from '../presence/presenceModel';
15-
import { getUserStatus } from '../user-statuses/userStatusesModel';
9+
import { getUserLastActiveAsRelativeTimeString } from '../presence/presenceModel';
1610
import ZulipText from '../common/ZulipText';
1711

1812
type Props = $ReadOnly<{|
@@ -21,19 +15,14 @@ type Props = $ReadOnly<{|
2115
|}>;
2216

2317
export default function ActivityText(props: Props): Node {
24-
const { style } = props;
18+
const { style, user } = props;
2519

26-
const zulipFeatureLevel = useSelector(getZulipFeatureLevel);
27-
const presence = useSelector(state =>
28-
getUserPresenceByEmail(getPresence(state), props.user.email),
20+
const activeTime = useSelector(state =>
21+
getUserLastActiveAsRelativeTimeString(state, user, Date.now()),
2922
);
30-
const userStatus = useSelector(state => getUserStatus(state, props.user.user_id));
31-
32-
if (!presence) {
23+
if (activeTime == null) {
3324
return null;
3425
}
3526

36-
const activeTime = userLastActiveAsRelativeTimeString(presence, userStatus, zulipFeatureLevel);
37-
3827
return <ZulipText style={style} text={`Active ${activeTime}`} />;
3928
}

0 commit comments

Comments
 (0)