Crunchify shell for avatar URLs, handle user_avatar_url_field_optional client capability

src/__tests__/lib/exampleData.js

@@ -14,6 +14,7 @@ import type {
 } from '../../api/modelTypes';
 import type { Action, GlobalState, RealmState } from '../../reduxTypes';
 import type { Auth, Account, Outbox } from '../../types';
+import { UploadedAvatarURL } from '../../utils/avatar';
 import { ZulipVersion } from '../../utils/zulipVersion';
 import {
   ACCOUNT_SWITCH,
@@ -88,7 +89,10 @@ const userOrBotProperties = ({ name: _name }) => {
   const name = _name ?? randString();
   const capsName = name.substring(0, 1).toUpperCase() + name.substring(1);
   return deepFreeze({
-    avatar_url: `https://zulip.example.org/yo/avatar-${name}.png`,
+    avatar_url: UploadedAvatarURL.validateAndConstructInstance({
+      realm: new URL('https://zulip.example.org'),
+      absoluteOrRelativeUrl: `/yo/avatar-${name}.png`,
+    }),
 
     date_joined: `2014-04-${randInt(30)
       .toString()
@@ -266,12 +270,11 @@ const messagePropertiesBase = deepFreeze({
 });
 
 const messagePropertiesFromSender = (user: User) => {
-  const { avatar_url, user_id: sender_id, email: sender_email, full_name: sender_full_name } = user;
+  const { user_id: sender_id, email: sender_email, full_name: sender_full_name } = user;
 
   return deepFreeze({
     sender_domain: '',
-
-    avatar_url,
+    avatar_url: user.avatar_url,
     client: 'ExampleClient',
     gravatar_hash: 'd3adb33f',
     sender_email,
@@ -366,7 +369,6 @@ export const streamMessage = (args?: {|
 const outboxMessageBase: $Diff<Outbox, {| id: mixed, timestamp: mixed |}> = deepFreeze({
   isOutbox: true,
   isSent: false,
-
   avatar_url: selfUser.avatar_url,
   content: '<p>Test.</p>',
   display_recipient: stream.name,

src/account-info/AccountDetails.js

@@ -6,10 +6,9 @@ import type { UserOrBot, Dispatch } from '../types';
 import styles, { createStyleSheet } from '../styles';
 import { connect } from '../react-redux';
 import { UserAvatar, ComponentList, RawLabel } from '../common';
-import { getCurrentRealm, getUserStatusTextForUser } from '../selectors';
+import { getUserStatusTextForUser } from '../selectors';
 import PresenceStatusIndicator from '../common/PresenceStatusIndicator';
 import ActivityText from '../title/ActivityText';
-import { getAvatarFromUser } from '../utils/avatar';
 import { nowInTimeZone } from '../utils/date';
 
 const componentStyles = createStyleSheet({
@@ -30,10 +29,7 @@ const componentStyles = createStyleSheet({
   },
 });
 
-const AVATAR_SIZE = 200;
-
 type SelectorProps = {|
-  realm: URL,
   userStatusText: string | void,
 |};
 
@@ -46,7 +42,7 @@ type Props = $ReadOnly<{|
 
 class AccountDetails extends PureComponent<Props> {
   render() {
-    const { realm, user, userStatusText } = this.props;
+    const { user, userStatusText } = this.props;
 
     let localTime: string | null = null;
     // See comments at CrossRealmBot and User at src/api/modelTypes.js.
@@ -62,7 +58,7 @@ class AccountDetails extends PureComponent<Props> {
     return (
       <ComponentList outerSpacing itemStyle={componentStyles.componentListItem}>
         <View>
-          <UserAvatar avatarUrl={getAvatarFromUser(user, realm, AVATAR_SIZE)} size={AVATAR_SIZE} />
+          <UserAvatar avatarUrl={user.avatar_url} size={200} />
         </View>
         <View style={componentStyles.statusWrapper}>
           <PresenceStatusIndicator
@@ -89,6 +85,5 @@ class AccountDetails extends PureComponent<Props> {
 }
 
 export default connect<SelectorProps, _, _>((state, props) => ({
-  realm: getCurrentRealm(state),
   userStatusText: getUserStatusTextForUser(state, props.user.user_id),
 }))(AccountDetails);

src/actionTypes.js

@@ -414,9 +414,11 @@ type EventUserRemoveAction = {|
 |};
 
 type EventUserUpdateAction = {|
+  ...ServerEvent,
   type: typeof EVENT_USER_UPDATE,
-  // In reality there's more -- but this will prevent accidentally using
-  // the type before going and adding those other properties here properly.
+  userId: number,
+  // Include only the fields that should be overwritten.
+  person: $Shape<User>,
 |};
 
 type EventMutedTopicsAction = {|

src/api/initialDataTypes.js

@@ -100,19 +100,26 @@ export type InitialDataRealmFilters = {|
   realm_filters: RealmFilter[],
 |};
 
-export type InitialDataRealmUser = {|
+export type RawInitialDataRealmUser = {|
   avatar_source: 'G',
   avatar_url: string | null,
   avatar_url_medium: string,
   can_create_streams: boolean,
-  cross_realm_bots: CrossRealmBot[],
+  cross_realm_bots: Array<{| ...CrossRealmBot, avatar_url?: string | null |}>,
   email: string,
   enter_sends: boolean,
   full_name: string,
   is_admin: boolean,
+  realm_users: Array<{| ...User, avatar_url?: string | null |}>,
+  realm_non_active_users: Array<{| ...User, avatar_url?: string | null |}>,
+  user_id: number,
+|};
+
+export type InitialDataRealmUser = {|
+  ...RawInitialDataRealmUser,
+  cross_realm_bots: CrossRealmBot[],
   realm_non_active_users: User[],
   realm_users: User[],
-  user_id: number,
 |};
 
 export type InitialDataRealmUserGroups = {|
@@ -280,7 +287,8 @@ export type InitialDataUserStatus = {|
   user_status?: UserStatusMapObject,
 |};
 
-// Initial data snapshot sent in response to a `/register` request.
+// Initial data snapshot sent in response to a `/register` request,
+// after validation and transformation.
 export type InitialData = {|
   // The server sends different subsets of the full available data,
   // depending on what event types the client subscribes to with the
@@ -306,3 +314,10 @@ export type InitialData = {|
   ...InitialDataUpdateMessageFlags,
   ...InitialDataUserStatus,
 |};
+
+// Initial data snapshot sent in response to a `/register` request,
+// before validation and transformation.
+export type RawInitialData = {|
+  ...InitialData,
+  ...RawInitialDataRealmUser,
+|};

src/api/messages/__tests__/migrateMessages-test.js

@@ -4,6 +4,7 @@ import { identityOfAuth } from '../../../account/accountMisc';
 import * as eg from '../../../__tests__/lib/exampleData';
 import type { ServerMessage, ServerReaction } from '../getMessages';
 import type { Message } from '../../modelTypes';
+import { GravatarURL } from '../../../utils/avatar';
 
 describe('migrateMessages', () => {
   const reactingUser = eg.makeUser();
@@ -22,6 +23,7 @@ describe('migrateMessages', () => {
   const serverMessage: ServerMessage = {
     ...eg.streamMessage(),
     reactions: [serverReaction],
+    avatar_url: null,
   };
 
   const input: ServerMessage[] = [serverMessage];
@@ -37,12 +39,17 @@ describe('migrateMessages', () => {
           emoji_code: serverReaction.emoji_code,
         },
       ],
+      avatar_url: GravatarURL.validateAndConstructInstance({ email: serverMessage.sender_email }),
     },
   ];
 
   const actualOutput: Message[] = migrateMessages(input, identityOfAuth(eg.selfAuth));
 
-  test('Replace user object with `user_id`', () => {
+  test('In reactions, replace user object with `user_id`', () => {
     expect(actualOutput.map(m => m.reactions)).toEqual(expectedOutput.map(m => m.reactions));
   });
+
+  test('Converts avatar_url correctly', () => {
+    expect(actualOutput.map(m => m.avatar_url)).toEqual(expectedOutput.map(m => m.avatar_url));
+  });
 });

src/api/messages/getMessages.js

@@ -5,6 +5,7 @@ import type { Message, Narrow } from '../apiTypes';
 import type { Reaction } from '../modelTypes';
 import { apiGet } from '../apiFetch';
 import { identityOfAuth } from '../../account/accountMisc';
+import { AvatarURL } from '../../utils/avatar';
 
 type ApiResponseMessages = {|
   ...ApiResponseSuccess,
@@ -32,6 +33,7 @@ export type ServerReaction = $ReadOnly<{|
 
 export type ServerMessage = $ReadOnly<{|
   ...$Exact<Message>,
+  avatar_url: string | null,
   reactions: $ReadOnlyArray<ServerReaction>,
 |}>;
 
@@ -45,9 +47,16 @@ type ServerApiResponseMessages = {|
 /** Exported for tests only. */
 export const migrateMessages = (messages: ServerMessage[], identity: Identity): Message[] =>
   messages.map(message => {
-    const { reactions, ...restMessage } = message;
+    const { reactions, avatar_url: rawAvatarUrl, ...restMessage } = message;
+
     return {
       ...restMessage,
+      avatar_url: AvatarURL.fromUserOrBotData({
+        rawAvatarUrl,
+        email: message.sender_email,
+        userId: message.sender_id,
+        realm: identity.realm,
+      }),
       reactions: reactions.map(reaction => {
         const { user, ...restReaction } = reaction;
         return {
@@ -92,6 +101,7 @@ export default async (
     num_after: numAfter,
     apply_markdown: true,
     use_first_unread_anchor: useFirstUnread,
+    client_gravatar: true,
   });
   return migrateResponse(response, identityOfAuth(auth));
 };

src/api/modelTypes.js

@@ -12,6 +12,8 @@
 //
 //
 
+import type { AvatarURL } from '../utils/avatar';
+
 export type ImageEmojiType = $ReadOnly<{|
   author?: $ReadOnly<{|
     email: string,
@@ -106,8 +108,22 @@ export type User = {|
   // instead of an empty string.
   timezone?: string,
 
-  // avatar_url is synthesized on the server by `get_avatar_field`.
-  avatar_url: string | null,
+  /**
+   * Present under EVENT_USER_ADD, EVENT_USER_UPDATE (if change
+   * indicated), under REALM_INIT, and in `state.users`, all as an
+   * AvatarURL, because we translate into that form at the edge.
+   *
+   * For how it appears at the edge (and how we translate) see
+   * AvatarURL.fromUserOrBotData.
+   */
+  avatar_url: AvatarURL,
+
+  // These properties appear in data from the server, but we ignore
+  // them. If we add these, we should try to avoid `avatar_url`
+  // falling out of sync with them.
+  // avatar_source: mixed,
+  // avatar_url_medium: mixed,
+  // avatar_version: mixed,
 
   // profile_data added in commit 02b845336 (in 1.8.0);
   // see also e3aed0f7b (in 2.0.0)
@@ -128,9 +144,10 @@ export type User = {|
  *  * `UserOrBot`, a convenience union
  */
 export type CrossRealmBot = {|
-  // avatar_url included since commit 58ee3fa8c (in 1.9.0)
-  // TODO(crunchy): convert missing -> null
-  avatar_url?: string | null,
+  /**
+   * See note for this property on User.
+   */
+  avatar_url: AvatarURL,
 
   // date_joined included since commit 58ee3fa8c (in 1.9.0)
   date_joined?: string,
@@ -480,10 +497,18 @@ export type Message = $ReadOnly<{|
    */
   flags?: $ReadOnlyArray<string>,
 
-  //
+  /**
+   * Present under EVENT_NEW_MESSAGE and under MESSAGE_FETCH_COMPLETE,
+   * and in `state.messages`, all as an AvatarURL, because we
+   * translate into that form at the edge.
+   *
+   * For how it appears at the edge (and how we translate) see
+   * AvatarURL.fromUserOrBotData.
+   */
+  avatar_url: AvatarURL,
+
   // The rest are believed to really appear in `message` events.
 
-  avatar_url: string | null,
   client: string,
   content: string,
   content_type: 'text/html' | 'text/markdown',

src/api/registerForEvents.js

@@ -1,8 +1,10 @@
 /* @flow strict-local */
-import type { InitialData } from './initialDataTypes';
+import type { RawInitialData, InitialData } from './initialDataTypes';
 import type { Auth } from './transportTypes';
 import type { Narrow } from './apiTypes';
+import type { CrossRealmBot, User } from './modelTypes';
 import { apiPost } from './apiFetch';
+import { AvatarURL } from '../utils/avatar';
 
 type RegisterForEventsParams = {|
   apply_markdown?: boolean,
@@ -16,17 +18,56 @@ type RegisterForEventsParams = {|
   client_capabilities?: {|
     notification_settings_null: boolean,
     bulk_message_deletion: boolean,
+    user_avatar_url_field_optional: boolean,
   |},
 |};
 
+const transformUser = (rawUser: {| ...User, avatar_url?: string | null |}, realm: URL): User => {
+  const { avatar_url: rawAvatarUrl, email } = rawUser;
+
+  return {
+    ...rawUser,
+    avatar_url: AvatarURL.fromUserOrBotData({
+      rawAvatarUrl,
+      email,
+      userId: rawUser.user_id,
+      realm,
+    }),
+  };
+};
+
+const transformCrossRealmBot = (
+  rawCrossRealmBot: {| ...CrossRealmBot, avatar_url?: string | null |},
+  realm: URL,
+): CrossRealmBot => {
+  const { avatar_url: rawAvatarUrl, user_id: userId, email } = rawCrossRealmBot;
+
+  return {
+    ...rawCrossRealmBot,
+    avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm }),
+  };
+};
+
+const transform = (rawInitialData: RawInitialData, auth: Auth): InitialData => ({
+  ...rawInitialData,
+  realm_users: rawInitialData.realm_users.map(rawUser => transformUser(rawUser, auth.realm)),
+  realm_non_active_users: rawInitialData.realm_non_active_users.map(rawNonActiveUser =>
+    transformUser(rawNonActiveUser, auth.realm),
+  ),
+  cross_realm_bots: rawInitialData.cross_realm_bots.map(rawCrossRealmBot =>
+    transformCrossRealmBot(rawCrossRealmBot, auth.realm),
+  ),
+});
+
 /** See https://zulip.com/api/register-queue */
 export default async (auth: Auth, params: RegisterForEventsParams): Promise<InitialData> => {
   const { narrow, event_types, fetch_event_types, client_capabilities } = params;
-  return apiPost(auth, 'register', {
+  const rawInitialData = await apiPost(auth, 'register', {
     ...params,
     narrow: JSON.stringify(narrow),
     event_types: JSON.stringify(event_types),
     fetch_event_types: JSON.stringify(fetch_event_types),
     client_capabilities: JSON.stringify(client_capabilities),
   });
+  return transform(rawInitialData, auth);
 };

src/api/users/getUsers.js

@@ -5,9 +5,13 @@ import { apiGet } from '../apiFetch';
 
 type ApiResponseUsers = {|
   ...ApiResponseSuccess,
-  members: User[],
+  members: $ReadOnlyArray<{| ...User, avatar_url: string | null |}>,
 |};
 
+// TODO: If we start to use this, we need to convert `.avatar_url` to
+// an AvatarURL instance, like we do in `registerForEvents` and
+// `EVENT_USER_ADD` and `EVENT_USER_UPDATE`.
+
 /** See https://zulip.com/api/get-all-users */
 export default (auth: Auth): Promise<ApiResponseUsers> =>
   apiGet(auth, 'users', { client_gravatar: true });