presence [nfc]: Modelify statusFromPresence, as getPresenceOnlyStatusForUser

This new function takes the whole PresenceState as an argument,
instead of a small fragment of it, which will help us as we add
new wrinkles to that state for #5669.

It also returns null for missing data, rather than picking a value;
it's then up to the caller to choose how to handle that.

Author: gnprice

src/presence/__tests__/presenceModel-test.js

@@ -8,9 +8,9 @@ import { PRESENCE_RESPONSE, EVENT_PRESENCE } from '../../actionConstants';
 import {
   reducer as presenceReducer,
   getAggregatedPresence,
-  statusFromPresence,
   getUserLastActiveAsRelativeTimeString,
   getPresenceStatusForUserId,
+  getPresenceOnlyStatusForUser,
 } from '../presenceModel';
 import { makePresenceState } from './presence-testlib';
 import type { UserPresence, UserStatus } from '../../api/modelTypes';
@@ -144,48 +144,52 @@ describe('getUserLastActiveAsRelativeTimeString', () => {
   });
 });
 
-describe('statusFromPresence', () => {
+describe('getPresenceOnlyStatusForUser', () => {
   test('if aggregate status is "offline" the result is always "offline"', () => {
+    const userPresence = {
+      aggregated: { client: 'website', status: 'offline', timestamp: currentTimestamp - 100 },
+    };
     expect(
-      statusFromPresence({
-        aggregated: { client: 'website', status: 'offline', timestamp: currentTimestamp - 100 },
-      }),
+      getPresenceOnlyStatusForUser(makePresenceState([[eg.otherUser, userPresence]]), eg.otherUser),
     ).toBe('offline');
   });
 
   test('if status is not "offline" but last activity was more than 1hr ago result is still "offline"', () => {
+    const userPresence = {
+      aggregated: {
+        client: 'website',
+        status: 'active',
+        timestamp: Math.trunc(Date.now() / 1000 - 2 * 60 * 60), // two hours
+      },
+    };
     expect(
-      statusFromPresence({
-        aggregated: {
-          client: 'website',
-          status: 'active',
-          timestamp: Math.trunc(Date.now() / 1000 - 2 * 60 * 60), // two hours
-        },
-      }),
+      getPresenceOnlyStatusForUser(makePresenceState([[eg.otherUser, userPresence]]), eg.otherUser),
     ).toBe('offline');
   });
 
   test('if status is  "idle" and last activity is less than 140 seconds then result remain "idle"', () => {
+    const userPresence = {
+      aggregated: {
+        client: 'website',
+        status: 'idle',
+        timestamp: Math.trunc(Date.now() / 1000 - 60), // 1 minute
+      },
+    };
     expect(
-      statusFromPresence({
-        aggregated: {
-          client: 'website',
-          status: 'idle',
-          timestamp: Math.trunc(Date.now() / 1000 - 60), // 1 minute
-        },
-      }),
+      getPresenceOnlyStatusForUser(makePresenceState([[eg.otherUser, userPresence]]), eg.otherUser),
     ).toBe('idle');
   });
 
   test('if status is not "offline" and last activity was less than 5min ago result is "active"', () => {
+    const userPresence = {
+      aggregated: {
+        client: 'website',
+        status: 'active',
+        timestamp: Math.trunc(Date.now() / 1000 - 60), // 1 minute
+      },
+    };
     expect(
-      statusFromPresence({
-        aggregated: {
-          client: 'website',
-          status: 'active',
-          timestamp: Math.trunc(Date.now() / 1000 - 60), // 1 minute
-        },
-      }),
+      getPresenceOnlyStatusForUser(makePresenceState([[eg.otherUser, userPresence]]), eg.otherUser),
     ).toBe('active');
   });
 });

src/presence/presenceModel.js

@@ -33,7 +33,7 @@ import type { UserOrBot } from '../api/modelTypes';
 
 export const getPresence = (state: PerAccountState): PresenceState => state.presence;
 
-export function getUserPresenceByEmail(state: PresenceState, email: string): UserPresence | void {
+function getUserPresenceByEmail(state: PresenceState, email: string): UserPresence | void {
   return state.byEmail.get(email);
 }
 
@@ -143,24 +143,29 @@ export function getUserLastActiveAsRelativeTimeString(
     : `${formatDistanceToNow(lastTimeActive)} ago`;
 }
 
-export const statusFromPresence = (presence: UserPresence | void): PresenceStatus => {
-  if (!presence || !presence.aggregated) {
-    return 'offline';
+export function getPresenceOnlyStatusForUser(
+  state: PresenceState,
+  user: UserOrBot,
+): PresenceStatus | null {
+  const userPresence = getUserPresenceByEmail(state, user.email);
+  if (!userPresence || !userPresence.aggregated) {
+    return null;
   }
+  const { status, timestamp } = userPresence.aggregated;
 
-  if (presence.aggregated.status === 'offline') {
+  if (status === 'offline') {
     return 'offline';
   }
 
-  const timestampDate = new Date(presence.aggregated.timestamp * 1000);
+  const timestampDate = new Date(timestamp * 1000);
   const diffToNowInSeconds = differenceInSeconds(Date.now(), timestampDate);
 
   if (diffToNowInSeconds > OFFLINE_THRESHOLD_SECS) {
     return 'offline';
   }
 
-  return presence.aggregated.status;
-};
+  return status;
+}
 
 /**
  * Get a user's overall presence status, aggregated from all their devices.
@@ -183,12 +188,8 @@ export const getPresenceStatusForUserId = (
   if (!user) {
     return null;
   }
-  const userPresence = getUserPresenceByEmail(presence, user.email);
-  if (!userPresence || !userPresence.aggregated) {
-    return null;
-  }
 
-  return statusFromPresence(userPresence);
+  return getPresenceOnlyStatusForUser(presence, user);
 };
 
 //

src/users/userHelpers.js

@@ -5,14 +5,14 @@ import * as typeahead from '@zulip/shared/lib/typeahead';
 
 import type {
   MutedUsersState,
-  UserPresence,
   UserId,
   UserGroup,
   PresenceState,
+  PresenceStatus,
   UserOrBot,
 } from '../types';
 import { ensureUnreachable } from '../types';
-import { getUserPresenceByEmail, statusFromPresence } from '../presence/presenceModel';
+import { getPresenceOnlyStatusForUser } from '../presence/presenceModel';
 
 type UsersByStatus = {|
   active: UserOrBot[],
@@ -27,14 +27,13 @@ export const groupUsersByStatus = (
 ): UsersByStatus => {
   const groupedUsers = { active: [], idle: [], offline: [], unavailable: [] };
   users.forEach(user => {
-    const status = statusFromPresence(getUserPresenceByEmail(presences, user.email));
+    const status = getPresenceOnlyStatusForUser(presences, user) ?? 'offline';
     groupedUsers[status].push(user);
   });
   return groupedUsers;
 };
 
-const statusOrder = (presence: UserPresence | void): number => {
-  const status = statusFromPresence(presence);
+const statusOrder = (status: PresenceStatus): number => {
   switch (status) {
     case 'active':
       return 1;
@@ -54,8 +53,8 @@ export const sortUserList = (
 ): $ReadOnlyArray<UserOrBot> =>
   [...users].sort(
     (x1, x2) =>
-      statusOrder(getUserPresenceByEmail(presences, x1.email))
-        - statusOrder(getUserPresenceByEmail(presences, x2.email))
+      statusOrder(getPresenceOnlyStatusForUser(presences, x1) ?? 'offline')
+        - statusOrder(getPresenceOnlyStatusForUser(presences, x2) ?? 'offline')
       || x1.full_name.toLowerCase().localeCompare(x2.full_name.toLowerCase()),
   );