Skip to content

Commit f371ef2

Browse files
committed
api types [nfc]: Add rawModelTypes.js, for getMessages's ServerMessage, etc.
getMessages is a binding for the GET /messages endpoint. We're about to add a binding for the GET /message/{message_id} endpoint, which since zulip/zulip@82837304e (FL 120) has given us a full message object in the same shape. For both bindings, we want: (a) a type to describe the data given by the server to represent a message (ServerMessage before this commit), and (b) a function to transform that data into our Message type (migrateMessage before this commit). And it's good to deduplicate both (a) and (b), in part because any differences would be surprising and warrant investigation; see discussion: #5465 (comment) The old code was already clear that it was about fetched data, not data from events, because it was in the file that defined api.getMessages. Keep that meaning intact by making the names more specific: ServerMessage → FetchedMessage (the "server" part is clear from the new file's name) migrateMessage → transformFetchedMessage (the orthogonal change from "migrate" to "transform" should help prevent confusion with src/storage/migrations.js, which has nothing to do with what this function does.) It's useful to remember that this code was developed specifically for these fetch-message(s) endpoints, e.g., for a future investigation aiming to deduplicate most of the corresponding 'message'-event code with this code. (That last bit reminds me that we don't even have a deliberate type for how we expect servers to represent a message in 'message' events. I'll write a TODO for that in the next commit.)
1 parent 8689ab8 commit f371ef2

File tree

5 files changed

+226
-203
lines changed

5 files changed

+226
-203
lines changed
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/* @flow strict-local */
2+
import {
3+
transformFetchedMessage,
4+
type FetchedMessage,
5+
type FetchedReaction,
6+
} from '../rawModelTypes';
7+
import { identityOfAuth } from '../../account/accountMisc';
8+
import * as eg from '../../__tests__/lib/exampleData';
9+
import type { Message } from '../modelTypes';
10+
import { GravatarURL } from '../../utils/avatar';
11+
12+
describe('transformFetchedMessage', () => {
13+
const reactingUser = eg.makeUser();
14+
15+
const serverReaction: FetchedReaction = {
16+
emoji_name: '+1',
17+
reaction_type: 'unicode_emoji',
18+
emoji_code: '1f44d',
19+
user: {
20+
email: reactingUser.email,
21+
full_name: reactingUser.full_name,
22+
id: reactingUser.user_id,
23+
},
24+
};
25+
26+
const fetchedMessage: FetchedMessage = {
27+
...eg.streamMessage(),
28+
reactions: [serverReaction],
29+
avatar_url: null,
30+
edit_history: [
31+
{
32+
prev_content: 'foo',
33+
prev_rendered_content: '<p>foo</p>',
34+
prev_stream: eg.stream.stream_id,
35+
prev_topic: 'bar',
36+
stream: eg.otherStream.stream_id,
37+
timestamp: 0,
38+
topic: 'bar!',
39+
user_id: eg.selfUser.user_id,
40+
},
41+
],
42+
};
43+
44+
describe('recent server', () => {
45+
const input: FetchedMessage = fetchedMessage;
46+
47+
const expectedOutput: Message = {
48+
...fetchedMessage,
49+
reactions: [
50+
{
51+
user_id: reactingUser.user_id,
52+
emoji_name: serverReaction.emoji_name,
53+
reaction_type: serverReaction.reaction_type,
54+
emoji_code: serverReaction.emoji_code,
55+
},
56+
],
57+
avatar_url: GravatarURL.validateAndConstructInstance({ email: fetchedMessage.sender_email }),
58+
edit_history:
59+
// $FlowIgnore[incompatible-cast] - See MessageEdit type
60+
(fetchedMessage.edit_history: Message['edit_history']),
61+
};
62+
63+
const actualOutput: Message = transformFetchedMessage<Message>(
64+
input,
65+
identityOfAuth(eg.selfAuth),
66+
eg.recentZulipFeatureLevel,
67+
true,
68+
);
69+
70+
test('In reactions, replace user object with `user_id`', () => {
71+
expect(actualOutput.reactions).toEqual(expectedOutput.reactions);
72+
});
73+
74+
test('Converts avatar_url correctly', () => {
75+
expect(actualOutput.avatar_url).toEqual(expectedOutput.avatar_url);
76+
});
77+
78+
test('Keeps edit_history, if allowEditHistory is true', () => {
79+
expect(actualOutput.edit_history).toEqual(expectedOutput.edit_history);
80+
});
81+
82+
test('Drops edit_history, if allowEditHistory is false', () => {
83+
expect(
84+
transformFetchedMessage<Message>(
85+
input,
86+
identityOfAuth(eg.selfAuth),
87+
eg.recentZulipFeatureLevel,
88+
false,
89+
).edit_history,
90+
).toEqual(null);
91+
});
92+
});
93+
94+
describe('drops edit_history from pre-118 server', () => {
95+
expect(
96+
transformFetchedMessage<Message>(
97+
{
98+
...fetchedMessage,
99+
edit_history: [
100+
{
101+
prev_content: 'foo',
102+
prev_rendered_content: '<p>foo</p>',
103+
prev_stream: eg.stream.stream_id,
104+
prev_subject: 'bar',
105+
timestamp: 0,
106+
user_id: eg.selfUser.user_id,
107+
},
108+
],
109+
},
110+
identityOfAuth(eg.selfAuth),
111+
117,
112+
true,
113+
).edit_history,
114+
).toEqual(null);
115+
});
116+
});

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

Lines changed: 0 additions & 107 deletions
This file was deleted.

src/api/messages/getMessages.js

Lines changed: 4 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22
import type { Auth, ApiResponseSuccess } from '../transportTypes';
33
import type { Identity } from '../../types';
44
import type { Message, ApiNarrow } from '../apiTypes';
5-
import type { PmMessage, StreamMessage, Reaction, UserId, MessageEdit } from '../modelTypes';
5+
import { transformFetchedMessage, type FetchedMessage } from '../rawModelTypes';
66
import { apiGet } from '../apiFetch';
77
import { identityOfAuth } from '../../account/accountMisc';
8-
import { AvatarURL } from '../../utils/avatar';
98

109
type ApiResponseMessages = {|
1110
...$Exact<ApiResponseSuccess>,
@@ -16,104 +15,19 @@ type ApiResponseMessages = {|
1615
messages: $ReadOnlyArray<Message>,
1716
|};
1817

19-
/**
20-
* The variant of `Reaction` found in the actual server response.
21-
*
22-
* Note that reaction events have a *different* variation; see their
23-
* handling in `eventToAction`.
24-
*/
25-
// We shouldn't have to rely on this format on servers at feature
26-
// level 2+; those newer servers include a top-level `user_id` field
27-
// in addition to the `user` object. See #4072.
28-
// TODO(server-3.0): Simplify this away.
29-
export type ServerReaction = $ReadOnly<{|
30-
...$Diff<Reaction, {| user_id: mixed |}>,
31-
user: $ReadOnly<{|
32-
email: string,
33-
full_name: string,
34-
id: UserId,
35-
|}>,
36-
|}>;
37-
38-
/**
39-
* The elements of Message.edit_history found in the actual server response.
40-
*
41-
* Accurate for supported servers before and after FL 118. For convenience,
42-
* we drop objects of this type if the FL is <118, so that the modern shape
43-
* at Message.edit_history is the only shape we store in Redux; see there.
44-
*/
45-
// TODO(server-5.0): Simplify this away.
46-
export type ServerMessageEdit = $ReadOnly<{|
47-
prev_content?: string,
48-
prev_rendered_content?: string,
49-
prev_rendered_content_version?: number,
50-
prev_stream?: number,
51-
prev_topic?: string, // New in FL 118, replacing `prev_subject`.
52-
prev_subject?: string, // Replaced in FL 118 by `prev_topic`.
53-
stream?: number, // New in FL 118.
54-
timestamp: number,
55-
topic?: string, // New in FL 118.
56-
user_id: UserId | null,
57-
|}>;
58-
59-
// How `ServerMessage` relates to `Message`, in a way that applies
60-
// uniformly to `Message`'s subtypes.
61-
type ServerMessageOf<M: Message> = $ReadOnly<{|
62-
...$Exact<M>,
63-
avatar_url: string | null,
64-
reactions: $ReadOnlyArray<ServerReaction>,
65-
66-
// Unlike Message['edit_history'], this can't be `null`.
67-
edit_history?: $ReadOnlyArray<ServerMessageEdit>,
68-
|}>;
69-
70-
export type ServerMessage = ServerMessageOf<PmMessage> | ServerMessageOf<StreamMessage>;
71-
7218
// The actual response from the server. We convert the data from this to
7319
// `ApiResponseMessages` before returning it to application code.
7420
type ServerApiResponseMessages = {|
7521
...ApiResponseMessages,
76-
messages: $ReadOnlyArray<ServerMessage>,
22+
messages: $ReadOnlyArray<FetchedMessage>,
7723
|};
7824

79-
/** Exported for tests only. */
80-
export const migrateMessage = <M: Message>(
81-
message: ServerMessageOf<M>,
82-
identity: Identity,
83-
zulipFeatureLevel: number,
84-
allowEditHistory: boolean,
85-
): M => ({
86-
...message,
87-
avatar_url: AvatarURL.fromUserOrBotData({
88-
rawAvatarUrl: message.avatar_url,
89-
email: message.sender_email,
90-
userId: message.sender_id,
91-
realm: identity.realm,
92-
}),
93-
reactions: message.reactions.map(reaction => {
94-
const { user, ...restReaction } = reaction;
95-
return {
96-
...restReaction,
97-
user_id: user.id,
98-
};
99-
}),
100-
101-
// Why condition on allowEditHistory? See MessageBase['edit_history'].
102-
// Why FL 118 condition? See MessageEdit type.
103-
edit_history:
104-
/* eslint-disable operator-linebreak */
105-
allowEditHistory && zulipFeatureLevel >= 118
106-
? // $FlowIgnore[incompatible-cast] - See MessageEdit type
107-
(message.edit_history: $ReadOnlyArray<MessageEdit> | void)
108-
: null,
109-
});
110-
11125
const migrateResponse = (response, identity: Identity, zulipFeatureLevel, allowEditHistory) => {
11226
const { messages, ...restResponse } = response;
11327
return {
11428
...restResponse,
115-
messages: messages.map(<M: Message>(message: ServerMessageOf<M>): M =>
116-
migrateMessage(message, identity, zulipFeatureLevel, allowEditHistory),
29+
messages: messages.map(message =>
30+
transformFetchedMessage<Message>(message, identity, zulipFeatureLevel, allowEditHistory),
11731
),
11832
};
11933
};

0 commit comments

Comments
 (0)