diff --git a/src/api/registerForEvents.js b/src/api/registerForEvents.js index 5a0e379a199..def922d22e0 100644 --- a/src/api/registerForEvents.js +++ b/src/api/registerForEvents.js @@ -18,6 +18,7 @@ type RegisterForEventsParams = {| client_capabilities?: {| notification_settings_null: boolean, bulk_message_deletion: boolean, + user_avatar_url_field_optional: boolean, |}, |}; @@ -25,11 +26,11 @@ export const migrateUserOrBot = ( rawUserOrBot: $FlowFixMe, // server data pre-transformation realm: string, ): T => { - const { avatar_url: rawAvatarUrl, email } = rawUserOrBot; + const { avatar_url: rawAvatarUrl, user_id: userId, email } = rawUserOrBot; return { ...rawUserOrBot, - avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm }), + avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm }), }; }; diff --git a/src/boot/store.js b/src/boot/store.js index 9cc1c9994cf..f02e1cd4e44 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -11,7 +11,7 @@ import { persistStore, autoRehydrate } from '../third/redux-persist'; import type { Config } from '../third/redux-persist'; import { ZulipVersion } from '../utils/zulipVersion'; -import { GravatarURL, UploadedAvatarURL } from '../utils/avatar'; +import { GravatarURL, UploadedAvatarURL, FallbackAvatarURL } from '../utils/avatar'; import type { Action, GlobalState } from '../types'; import config from '../config'; import { REHYDRATE } from '../actionConstants'; @@ -279,6 +279,11 @@ const customReplacer = (key, value, defaultReplacer) => { data: UploadedAvatarURL.serialize(value), [SERIALIZED_TYPE_FIELD_NAME]: 'UploadedAvatarURL', }; + } else if (value instanceof FallbackAvatarURL) { + return { + data: FallbackAvatarURL.serialize(value), + [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL', + }; } return defaultReplacer(key, value); }; @@ -293,6 +298,8 @@ const customReviver = (key, value, defaultReviver) => { return GravatarURL.deserialize(data); case 'UploadedAvatarURL': return UploadedAvatarURL.deserialize(data); + case 'FallbackAvatarURL': + return FallbackAvatarURL.deserialize(data); default: // Fall back to defaultReviver, below } diff --git a/src/events/eventToAction.js b/src/events/eventToAction.js index 3e0ce270959..6df2d393fe2 100644 --- a/src/events/eventToAction.js +++ b/src/events/eventToAction.js @@ -132,21 +132,22 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => { switch (event.op) { case 'add': { - const { avatar_url: rawAvatarUrl, email } = event.person; + const { avatar_url: rawAvatarUrl, user_id: userId, email } = event.person; return { type: EVENT_USER_ADD, id: event.id, // TODO: Validate and rebuild `event.person`. person: { ...event.person, - avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm }), + avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm }), }, }; } case 'update': { + const { user_id: userId } = event.person; let existingUser: UserOrBot; try { - existingUser = getUserForId(state, event.person.user_id); + existingUser = getUserForId(state, userId); } catch (e) { // We don't store deactivated users. We don't know whether // these events are actually emitted for deactivated users @@ -161,13 +162,14 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => { return { type: EVENT_USER_UPDATE, id: event.id, - userId: event.person.user_id, + userId, // Just the fields we want to overwrite. person: { ...(event.person.avatar_url !== undefined ? { avatar_url: AvatarURL.fromUserOrBotData({ rawAvatarUrl: event.person.avatar_url, + userId, email: existingUser.email, realm, }), diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js index fff43cbecbe..6c201ec8bcd 100644 --- a/src/message/fetchActions.js +++ b/src/message/fetchActions.js @@ -308,6 +308,7 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat client_capabilities: { notification_settings_null: true, bulk_message_deletion: true, + user_avatar_url_field_optional: true, }, }), ), diff --git a/src/utils/__tests__/avatar-test.js b/src/utils/__tests__/avatar-test.js index 5d08279ee36..fb5c67bafe0 100644 --- a/src/utils/__tests__/avatar-test.js +++ b/src/utils/__tests__/avatar-test.js @@ -1,18 +1,18 @@ /* @flow strict-local */ import md5 from 'blueimp-md5'; -import { AvatarURL, GravatarURL, UploadedAvatarURL } from '../avatar'; +import { AvatarURL, GravatarURL, FallbackAvatarURL, UploadedAvatarURL } from '../avatar'; import * as eg from '../../__tests__/lib/exampleData'; describe('AvatarURL', () => { describe('fromUserOrBotData', () => { const user = eg.makeUser(); - const { email } = user; + const { email, user_id: userId } = user; const realm = eg.realm; test('gives a `GravatarURL` if `rawAvatarURL` is null', () => { const rawAvatarUrl = null; - expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf( + expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf( GravatarURL, ); }); @@ -20,7 +20,7 @@ describe('AvatarURL', () => { test('gives a `GravatarURL` if `rawAvatarURL` is a URL string on Gravatar origin', () => { const rawAvatarUrl = 'https://secure.gravatar.com/avatar/2efaec12efd9bea8a089299208117786?d=identicon&version=3'; - expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf( + expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf( GravatarURL, ); }); @@ -28,7 +28,7 @@ describe('AvatarURL', () => { test('gives an `UploadedAvatarURL` if `rawAvatarURL` is a non-Gravatar absolute URL string', () => { const rawAvatarUrl = 'https://zulip-avatars.s3.amazonaws.com/13/430713047f2cffed661f84e139a64f864f17f286?x=x&version=5'; - expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf( + expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf( UploadedAvatarURL, ); }); @@ -36,7 +36,7 @@ describe('AvatarURL', () => { test('gives an `UploadedAvatarURL` if `rawAvatarURL` is a relative URL string', () => { const rawAvatarUrl = '/user_avatars/2/08fb6d007eb10a56efee1d64760fbeb6111c4352.png?x=x&version=2'; - expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, email, realm })).toBeInstanceOf( + expect(AvatarURL.fromUserOrBotData({ rawAvatarUrl, userId, email, realm })).toBeInstanceOf( UploadedAvatarURL, ); }); @@ -181,3 +181,28 @@ describe('UploadedAvatarURL', () => { }); }); }); + +describe('FallbackAvatarURL', () => { + test('serializes/deserializes correctly', () => { + const instance = new FallbackAvatarURL({ + realm: new URL(eg.realm), + userId: eg.selfUser.user_id, + }); + + const roundTripped = FallbackAvatarURL.deserialize(FallbackAvatarURL.serialize(instance)); + + SIZES_WE_USE.forEach(size => { + expect(instance.get(size).toString()).toEqual(roundTripped.get(size).toString()); + }); + }); + + test('gives the `/avatar/{user_id}` URL, on the provided realm', () => { + const userId = eg.selfUser.user_id; + const instance = new FallbackAvatarURL({ + realm: new URL('https://chat.zulip.org'), + userId, + }); + + expect(instance.get().toString()).toEqual(`https://chat.zulip.org/avatar/${userId.toString()}`); + }); +}); diff --git a/src/utils/avatar.js b/src/utils/avatar.js index 10122288baf..78fb22de440 100644 --- a/src/utils/avatar.js +++ b/src/utils/avatar.js @@ -19,7 +19,8 @@ export class AvatarURL { * defined. */ static fromUserOrBotData: (args: {| - rawAvatarUrl: string | null, + rawAvatarUrl: string | void | null, + userId: number, email: string, realm: string, |}) => AvatarURL; @@ -100,6 +101,57 @@ export class GravatarURL extends AvatarURL { } } +/** + * The /avatar/{user_id} redirect. + * + * See the point on `user_avatar_url_field_optional` at + * https://zulipchat.com/api/register-queue. + * + * This endpoint does not currently support size customization. + */ +export class FallbackAvatarURL extends AvatarURL { + _url: URL; + + /** + * Private; only stored for use by serialize/deserialize. + */ + _constructorArgs: {| realm: URL, userId: number |}; + + /** + * Serialize to a special string; reversible with `deserialize`. + */ + static serialize(instance: FallbackAvatarURL): string { + // eslint-disable-next-line no-underscore-dangle + const instanceConstructorArgs = instance._constructorArgs; + return JSON.stringify({ + ...instanceConstructorArgs, + realm: instanceConstructorArgs.realm.toString(), + }); + } + + /** + * Use a special string from `serialize` to make a new instance. + */ + static deserialize(serialized: string): FallbackAvatarURL { + const parsedArgs = JSON.parse(serialized); + return new FallbackAvatarURL({ + ...parsedArgs, + realm: new URL(parsedArgs.realm), + }); + } + + constructor(args: typeof FallbackAvatarURL.prototype._constructorArgs) { + super(); + this._constructorArgs = args; + const { realm, userId } = args; + this._url = new URL(`/avatar/${userId.toString()}`, realm.origin); + } + + get(size: number | void): URL { + return this._url; + } +} + /** * An avatar that was uploaded locally, or to an S3 backend. * @@ -167,12 +219,26 @@ export class UploadedAvatarURL extends AvatarURL { } AvatarURL.fromUserOrBotData = (args: {| - rawAvatarUrl: string | null, + rawAvatarUrl: string | void | null, + userId: number, email: string, realm: string, |}): AvatarURL => { - const { rawAvatarUrl, email, realm } = args; - if (rawAvatarUrl === null) { + const { rawAvatarUrl, userId, email, realm } = args; + if (rawAvatarUrl === undefined) { + // New in Zulip 2.2, feature level 18, the field may be missing at + // the server's discretion, if we announce the + // `user_avatar_url_field_optional` client capability, which we + // do. See the note about `user_avatar_url_field_optional` at + // https://zulipchat.com/api/register-queue. + // + // It will also be absent on cross-realm bots from servers prior + // to 58ee3fa8c (1.9.0). The effect of using FallbackAvatarURL for + // this case isn't thoroughly considered, but at worst, it means a + // 404. We could plumb through the server version and + // conditionalize on that. + return new FallbackAvatarURL({ realm: new URL(realm), userId }); + } else if (rawAvatarUrl === null) { // If we announce `client_gravatar`, which we do, `rawAvatarUrl` // might be null. In that case, we take responsibility for // computing a hash for the user's email and using it to form a