Skip to content

Commit 7059199

Browse files
gnpricechrisbobbe
authored andcommitted
messagesActions [nfc]: Pull out getSingleMessage from adjustNarrowForMoves
This part of the logic is fairly self-contained and has a somewhat different character from the rest of it, so it seems helpful to separate out.
1 parent 0daeb12 commit 7059199

File tree

1 file changed

+68
-50
lines changed

1 file changed

+68
-50
lines changed

src/message/messagesActions.js

Lines changed: 68 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* @flow strict-local */
22
import * as logging from '../utils/logging';
33
import * as NavigationService from '../nav/NavigationService';
4-
import type { Narrow, ThunkAction } from '../types';
4+
import type { Message, Narrow, ThunkAction } from '../types';
55
import { getAuth, getRealm, getMessages, getZulipFeatureLevel } from '../selectors';
66
import { getNearOperandFromLink, getNarrowFromLink } from '../utils/internalLinks';
77
import { openLinkWithUserPreference } from '../utils/openLink';
@@ -32,6 +32,69 @@ export const doNarrow =
3232
NavigationService.dispatch(navigateToChat(narrow));
3333
};
3434

35+
/**
36+
* Find the given message, either locally or on the server.
37+
*
38+
* If we have the message locally, avoids any network request.
39+
*
40+
* Returns null if we can't find the message -- it doesn't exist, or our
41+
* user can't see it, or a server request for it failed, or the server was
42+
* simply too old (FL <120).
43+
*
44+
* N.B.: Gives a bad experience when the request takes a long time. We
45+
* should fix that; see TODOs.
46+
*/
47+
const getSingleMessage =
48+
(messageId: number): ThunkAction<Promise<Message | null>> =>
49+
async (dispatch, getState) => {
50+
const state = getState();
51+
52+
const auth = getAuth(state);
53+
const messages = getMessages(state);
54+
const zulipFeatureLevel = getZulipFeatureLevel(state);
55+
const allowEditHistory = getRealm(state).allowEditHistory;
56+
57+
// Try to get it from our local data to avoid a server round-trip…
58+
let message = messages.get(messageId);
59+
60+
// …but if we have to, go and ask the server.
61+
// TODO: Give feedback when the server round trip takes longer than
62+
// expected.
63+
// TODO: Let the user cancel the request so we don't force a doNarrow
64+
// after they've given up on tapping the link, and perhaps forgotten
65+
// about it. Like any request, this might take well over a minute to
66+
// resolve, or never resolve.
67+
// TODO: When these are fixed, remove warning in jsdoc.
68+
if (!message) {
69+
// TODO(server-5.0): Simplify; simplify jsdoc to match.
70+
if (zulipFeatureLevel < 120) {
71+
// api.getSingleMessage won't give us the message's stream and
72+
// topic; see there.
73+
return null;
74+
}
75+
try {
76+
message = await api.getSingleMessage(
77+
auth,
78+
{ message_id: messageId },
79+
zulipFeatureLevel,
80+
allowEditHistory,
81+
);
82+
} catch {
83+
return null;
84+
}
85+
}
86+
87+
// The FL 120 condition on calling api.getSingleMessage should ensure
88+
// `message` isn't void.
89+
// TODO(server-5.0): Simplify away.
90+
if (!message) {
91+
logging.error('`message` from api.getSingleMessage unexpectedly falsy');
92+
return null;
93+
}
94+
95+
return message;
96+
};
97+
3598
/**
3699
* Adjust a /near/ link as needed to account for message moves.
37100
*
@@ -47,20 +110,10 @@ export const doNarrow =
47110
* If those can't be gotten from Redux, we ask the server. If the server
48111
* can't help us (gives an error), we can't help the user, so we won't
49112
* follow a move in that case.
50-
*
51-
* N.B.: Gives a bad experience when the request takes a long time. We
52-
* should fix that; see TODOs.
53113
*/
54114
const adjustNarrowForMoves =
55115
(narrow: Narrow, nearOperand: number): ThunkAction<Promise<Narrow>> =>
56116
async (dispatch, getState) => {
57-
const state = getState();
58-
59-
const auth = getAuth(state);
60-
const messages = getMessages(state);
61-
const zulipFeatureLevel = getZulipFeatureLevel(state);
62-
const allowEditHistory = getRealm(state).allowEditHistory;
63-
64117
// Many control-flow paths here simply return the original narrow.
65118
//
66119
// We do this when the link is meant to find the specific message
@@ -97,45 +150,10 @@ const adjustNarrowForMoves =
97150
return narrow;
98151
}
99152

100-
// Grab the message and see if it was moved, so we can follow the move
101-
// if so.
102-
103-
// Try to get it from our local data to avoid a server round-trip…
104-
let message = messages.get(nearOperand);
105-
106-
// …but if we have to, go and ask the server.
107-
// TODO: Give feedback when the server round trip takes longer than
108-
// expected.
109-
// TODO: Let the user cancel the request so we don't force a doNarrow
110-
// after they've given up on tapping the link, and perhaps forgotten
111-
// about it. Like any request, this might take well over a minute to
112-
// resolve, or never resolve.
113-
// TODO: When these are fixed, remove warning in jsdoc.
153+
const message = await dispatch(getSingleMessage(nearOperand));
114154
if (!message) {
115-
// TODO(server-5.0): Simplify.
116-
if (zulipFeatureLevel < 120) {
117-
// api.getSingleMessage won't give us the message's stream and
118-
// topic; see there. Hopefully the message wasn't moved.
119-
return narrow;
120-
}
121-
try {
122-
message = await api.getSingleMessage(
123-
auth,
124-
{ message_id: nearOperand },
125-
zulipFeatureLevel,
126-
allowEditHistory,
127-
);
128-
} catch {
129-
// Hopefully the message, if it exists or ever existed, wasn't moved.
130-
return narrow;
131-
}
132-
}
133-
134-
// The FL 120 condition on calling api.getSingleMessage should ensure
135-
// `message` isn't void.
136-
// TODO(server-5.0): Simplify away.
137-
if (!message) {
138-
logging.error('`message` from api.getSingleMessage unexpectedly falsy');
155+
// We couldn't find the message. Hopefully it wasn't moved,
156+
// if it exists or ever did.
139157
return narrow;
140158
}
141159

@@ -186,7 +204,7 @@ const adjustNarrowForMoves =
186204
* See `adjustNarrowForMoves` for more discussion.
187205
*
188206
* N.B.: Gives a bad experience when the request takes a long time. We
189-
* should fix that; see `adjustNarrowForMoves`.
207+
* should fix that; see `getSingleMessage`.
190208
*/
191209
const doNarrowNearLink =
192210
(narrow: Narrow, nearOperand: number): ThunkAction<Promise<void>> =>

0 commit comments

Comments
 (0)