Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/__tests__/lib/exampleData.js
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@ export const action = deepFreeze({
realm_users: [],
user_id: 4,
realm_user_groups: [],
recent_private_conversations: [],
streams: [],
never_subscribed: [],
subscriptions: [],
Expand Down Expand Up @@ -592,6 +593,7 @@ export const action = deepFreeze({
numAfter: 50,
foundNewest: undefined,
foundOldest: undefined,
ownUserId: selfUser.user_id,
},
});

Expand Down
1 change: 1 addition & 0 deletions src/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ type MessageFetchCompleteAction = {|
numAfter: number,
foundNewest: boolean | void,
foundOldest: boolean | void,
ownUserId: number,
|};

type InitialFetchStartAction = {|
Expand Down
10 changes: 10 additions & 0 deletions src/api/initialDataTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
CrossRealmBot,
RealmEmojiById,
RealmFilter,
RecentPrivateConversation,
Stream,
Subscription,
User,
Expand Down Expand Up @@ -129,6 +130,14 @@ export type InitialDataRealmUserGroups = {|
realm_user_groups?: UserGroup[],
|};

export type InitialDataRecentPmConversations = {|
// * Added in server commit 2.1-dev-384-g4c3c669b41.
// * `user_id` fields are sorted as of commit 2.2-dev-53-g405a529340, which
// was backported to 2.1.1-50-gd452ad31e0 -- meaning that they are _not_
// sorted in either v2.1.0 or v2.1.1.
recent_private_conversations?: RecentPrivateConversation[],
|};

type NeverSubscribedStream = {|
description: string,
invite_only: boolean,
Expand Down Expand Up @@ -307,6 +316,7 @@ export type InitialData = {|
...InitialDataRealmFilters,
...InitialDataRealmUser,
...InitialDataRealmUserGroups,
...InitialDataRecentPmConversations,
...InitialDataStream,
...InitialDataSubscription,
...InitialDataUpdateDisplaySettings,
Expand Down
26 changes: 26 additions & 0 deletions src/api/modelTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -579,3 +579,29 @@ export type Message = $ReadOnly<{|
subject: string,
subject_links: $ReadOnlyArray<string>,
|}>;

//
//
//
// ===================================================================
// Summaries of messages and conversations.
//
//

/**
* Describes a single recent PM conversation.
*
* See API documentation under `recent_private_conversations` at:
* https://chat.zulip.org/api/register-queue
*
* Note that `user_ids` does not contain the `user_id` of the current user.
* Consequently, a user's conversation with themselves will be listed here
* as [], which is unlike the behaviour found in some other parts of the
* codebase.
*/
export type RecentPrivateConversation = {|
max_message_id: number,
// When received from the server, these are guaranteed to be sorted only after
// 2.2-dev-53-g405a529340. To be safe, we always sort them on receipt.
user_ids: number[],
|};
2 changes: 2 additions & 0 deletions src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ exports[`Stringify fallbackAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.exampl

exports[`Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.com/avatar/3b01d0f626dc6944ed45dbe6c86d3e30?d=identicon\\",\\"__serializedType__\\":\\"GravatarURL\\"}"`;

exports[`Stringify list 1`] = `"{\\"data\\":[1,2,\\"a\\",null],\\"__serializedType__\\":\\"ImmutableList\\"}"`;

exports[`Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`;

exports[`Stringify mapWithTypeKey 1`] = `"{\\"data\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}},\\"__serializedType__\\":\\"ImmutableMap\\"}"`;
Expand Down
1 change: 1 addition & 0 deletions src/boot/__tests__/replaceRevive-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { stringify, parse, SERIALIZED_TYPE_FIELD_NAME } from '../replaceRevive';
import * as eg from '../../__tests__/lib/exampleData';

const data = {
list: Immutable.List([1, 2, 'a', null]),
map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }),
mapWithTypeKey: Immutable.Map({
a: 1,
Expand Down
2 changes: 2 additions & 0 deletions src/boot/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import narrows from '../chat/narrowsReducer';
import messages from '../message/messagesReducer';
import mute from '../mute/muteReducer';
import outbox from '../outbox/outboxReducer';
import { reducer as pmConversations } from '../pm-conversations/pmConversationsModel';
import presence from '../presence/presenceReducer';
import realm from '../realm/realmReducer';
import session from '../session/sessionReducer';
Expand Down Expand Up @@ -46,6 +47,7 @@ const reducers = {
narrows,
mute,
outbox,
pmConversations,
presence,
realm,
session,
Expand Down
4 changes: 4 additions & 0 deletions src/boot/replaceRevive.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ function replacer(key, value) {
data: FallbackAvatarURL.serialize(value),
[SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL',
};
case (Immutable.List.prototype: $FlowFixMe):
return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableList' };
case (Immutable.Map.prototype: $FlowFixMe):
return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' };
default: {
Expand Down Expand Up @@ -132,6 +134,8 @@ function reviver(key, value) {
return UploadedAvatarURL.deserialize(data);
case 'FallbackAvatarURL':
return FallbackAvatarURL.deserialize(data);
case 'ImmutableList':
return Immutable.List(data);
case 'ImmutableMap':
return Immutable.Map(data);
case 'Object':
Expand Down
2 changes: 1 addition & 1 deletion src/boot/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const storeKeys: Array<$Keys<GlobalState>> = [
*/
// prettier-ignore
export const cacheKeys: Array<$Keys<GlobalState>> = [
'flags', 'messages', 'mute', 'narrows', 'realm', 'streams',
'flags', 'messages', 'mute', 'narrows', 'pmConversations', 'realm', 'streams',
'subscriptions', 'unread', 'userGroups', 'users',
];

Expand Down
5 changes: 5 additions & 0 deletions src/chat/__tests__/narrowsReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ describe('narrowsReducer', () => {
numAfter: 100,
foundNewest: false,
foundOldest: false,
ownUserId: eg.selfUser.user_id,
});

const expectedState = Immutable.Map({
Expand Down Expand Up @@ -407,6 +408,7 @@ describe('narrowsReducer', () => {
numAfter: 100,
foundNewest: false,
foundOldest: false,
ownUserId: eg.selfUser.user_id,
});

const expectedState = Immutable.Map({
Expand Down Expand Up @@ -434,6 +436,7 @@ describe('narrowsReducer', () => {
numAfter: 100,
foundNewest: false,
foundOldest: false,
ownUserId: eg.selfUser.user_id,
});

const expectedState = Immutable.Map({
Expand Down Expand Up @@ -461,6 +464,7 @@ describe('narrowsReducer', () => {
numAfter: 100,
foundNewest: false,
foundOldest: false,
ownUserId: eg.selfUser.user_id,
});

const expectedState = Immutable.Map({
Expand All @@ -487,6 +491,7 @@ describe('narrowsReducer', () => {
numAfter: 0,
foundNewest: true,
foundOldest: false,
ownUserId: eg.selfUser.user_id,
});

const expectedState = Immutable.Map({
Expand Down
1 change: 1 addition & 0 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const config: Config = {
'realm_filters',
'realm_user',
'realm_user_groups',
'recent_private_conversations',
'stream',
'subscription',
'update_display_settings',
Expand Down
4 changes: 4 additions & 0 deletions src/message/__tests__/fetchActions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ describe('fetchActions', () => {

const baseState = eg.reduxState({
accounts: [eg.makeAccount()],
realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
narrows: Immutable.Map({
[streamNarrowStr]: [message1.id],
}),
Expand Down Expand Up @@ -299,6 +300,7 @@ describe('fetchActions', () => {
const store = mockStore<GlobalState, Action>(
eg.reduxState({
accounts: [eg.selfAccount],
realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
narrows: Immutable.Map({
[streamNarrowStr]: [1],
}),
Expand All @@ -324,6 +326,7 @@ describe('fetchActions', () => {
const store = mockStore<GlobalState, Action>(
eg.reduxState({
accounts: [eg.selfAccount],
realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
narrows: Immutable.Map({
[streamNarrowStr]: [1],
}),
Expand All @@ -348,6 +351,7 @@ describe('fetchActions', () => {
const store = mockStore<GlobalState, Action>(
eg.reduxState({
accounts: [eg.selfAccount],
realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
narrows: Immutable.Map({
[streamNarrowStr]: [1],
}),
Expand Down
34 changes: 26 additions & 8 deletions src/message/fetchActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ import { realmInit } from '../realm/realmActions';
import { startEventPolling } from '../events/eventActions';
import { logout } from '../account/accountActions';
import { ZulipVersion } from '../utils/zulipVersion';
import { getAllUsersById } from '../users/userSelectors';
import { getAllUsersById, getOwnUserId } from '../users/userSelectors';
import { MIN_RECENTPMS_SERVER_VERSION } from '../pm-conversations/pmConversationsModel';

const messageFetchStart = (narrow: Narrow, numBefore: number, numAfter: number): Action => ({
type: MESSAGE_FETCH_START,
Expand All @@ -58,8 +59,18 @@ const messageFetchComplete = (args: {|
numAfter: number,
foundNewest?: boolean,
foundOldest?: boolean,
ownUserId: number,
|}): Action => {
const { messages, narrow, anchor, numBefore, numAfter, foundNewest, foundOldest } = args;
const {
messages,
narrow,
anchor,
numBefore,
numAfter,
foundNewest,
foundOldest,
ownUserId,
} = args;
return {
type: MESSAGE_FETCH_COMPLETE,
messages,
Expand All @@ -69,6 +80,7 @@ const messageFetchComplete = (args: {|
numAfter,
foundNewest,
foundOldest,
ownUserId,
};
};

Expand Down Expand Up @@ -98,6 +110,7 @@ export const fetchMessages = (fetchArgs: {|
messages,
foundNewest: found_newest,
foundOldest: found_oldest,
ownUserId: getOwnUserId(getState()),
}),
);
return messages;
Expand Down Expand Up @@ -227,13 +240,14 @@ export const fetchMessagesInNarrow = (
/**
* Fetch the few most recent PMs.
*
* We do this eagerly in `doInitialFetch`, where it mainly serves to let us
* show something useful in the PM conversations screen. Recent server
* versions have a custom-made API to help us do this better, which we hope
* to use soon: see #3133.
* For old servers, we do this eagerly in `doInitialFetch`, in order to
* let us show something useful in the PM conversations screen.
* Zulip Server 2.1 added a custom-made API to help us do this better;
* see #3133.
*
* See `fetchMessagesInNarrow` for further background.
*/
// TODO(server-2.1): Delete this.
const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState) => {
const auth = getAuth(getState());
const { messages, found_newest, found_oldest } = await api.getMessages(auth, {
Expand All @@ -251,6 +265,7 @@ const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState
numAfter: 0,
foundNewest: found_newest,
foundOldest: found_oldest,
ownUserId: getOwnUserId(getState()),
Copy link
Copy Markdown
Collaborator

@chrisbobbe chrisbobbe Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (here and in fetchMessages, above): From one perspective, this isn't NFC, right? getOwnUserId will throw with "No server data found" if the user has logged out, which might have happened while the API request was in progress. (Before this commit, that error would not have been thrown.)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, I don't think it's a very bad thing to throw here; the alternative is allowing race condition where the result of this message fetch is applied when the account it's meant for is no longer logged in.

Copy link
Copy Markdown
Collaborator

@chrisbobbe chrisbobbe Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, I don't think it's a very bad thing to throw here; the alternative is allowing race condition where the result of this message fetch is applied when the account it's meant for is no longer logged in.

Er, well, of course it's bad in that it would (I think) cause a crash—but having corrupted data is also quite bad and possibly harder to debug. 🙂 This is the kind of thing that we hope to work out with logic for cancelling in-progress requests when one logs out.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm true! Yeah, we should really fix this possible case in general by cancelling these requests (#4170), but we haven't yet.

I'll note this in the commit message.

}),
);
};
Expand Down Expand Up @@ -335,11 +350,14 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
return;
}

dispatch(realmInit(initData, new ZulipVersion(serverSettings.zulip_version)));
const serverVersion = new ZulipVersion(serverSettings.zulip_version);
dispatch(realmInit(initData, serverVersion));
dispatch(initialFetchComplete());
dispatch(startEventPolling(initData.queue_id, initData.last_event_id));

dispatch(fetchPrivateMessages());
if (!serverVersion.isAtLeast(MIN_RECENTPMS_SERVER_VERSION)) {
dispatch(fetchPrivateMessages());
}

dispatch(sendOutbox());
dispatch(initNotifications());
Expand Down
Loading