Skip to content

Commit e221fc1

Browse files
committed
messagesReducer [nfc]: Fix a sketchy null check.
This causes Flow to mark the `$FlowFixMe` on `EventUpdateMessageAction.orig_subject?` as an "unused suppression comment", so, remove it. In fact, I don't think a `$FlowFixMe` really belongs here; there's no question that `orig_subject` is sometimes missing, if only because private messages don't have topics. The problem is in working out when it's there and when it's not. As I point out in the ongoing discussion [1] linked from that TODO, there's a particular line [2] in the server code at current `master` that I believe ensures that `update_message` events won't come to us with an empty string for `orig_subject`. [1] https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/subject.20always.20present.20in.20event/near/1098954 [2] https://github.com/zulip/zulip/blob/08d716c74175a8b63bf8eb1080381c77f3f853c6/zerver/views/message_edit.py#L157
1 parent ac64b4e commit e221fc1

File tree

2 files changed

+16
-2
lines changed

2 files changed

+16
-2
lines changed

src/actionTypes.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,22 @@ type EventUpdateMessageAction = {|
324324
message_id: number,
325325
// TODO is it really right that just one of the orig_* is optional?
326326
orig_content: string,
327-
// $FlowFixMe clarify orig_subject vs. other orig_*, and missing vs. empty
327+
328+
// TODO: The doc for this field isn't yet correct; it turns out that
329+
// the question of whether `orig_subject` is present or not is
330+
// complicated; see discussion at
331+
// https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/subject.20always.20present.20in.20event/near/1098954.
332+
//
333+
// We can be pretty sure of a few things, though:
334+
// - it will not be present if the message doesn't have a topic
335+
// (i.e., if it's a private message)
336+
// - it's guaranteed to be present if the topic did indeed change
337+
// - it will never be an empty string, because the server doesn't
338+
// accept the empty string for a message's topic; it requires
339+
// clients to specify something like `(no topic)` if no topic is
340+
// desired.
328341
orig_subject?: string,
342+
329343
orig_rendered_content: string,
330344
prev_rendered_content_version: number,
331345
rendered_content: string,

src/message/messagesReducer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ export default (state: MessagesState = initialState, action: Action): MessagesSt
110110
subject_links: action.subject_links || oldMessage.subject_links,
111111
edit_history: [
112112
action.orig_rendered_content
113-
? action.orig_subject
113+
? action.orig_subject !== undefined
114114
? {
115115
prev_rendered_content: action.orig_rendered_content,
116116
prev_subject: oldMessage.subject,

0 commit comments

Comments
 (0)