Skip to content

Commit

Permalink
messagesReducer [nfc]: Fix a sketchy null check.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
chrisbobbe committed Jan 14, 2021
1 parent ac64b4e commit e221fc1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
16 changes: 15 additions & 1 deletion src/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,22 @@ type EventUpdateMessageAction = {|
message_id: number,
// TODO is it really right that just one of the orig_* is optional?
orig_content: string,
// $FlowFixMe clarify orig_subject vs. other orig_*, and missing vs. empty

// TODO: The doc for this field isn't yet correct; it turns out that
// the question of whether `orig_subject` is present or not is
// complicated; see discussion at
// https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/subject.20always.20present.20in.20event/near/1098954.
//
// We can be pretty sure of a few things, though:
// - it will not be present if the message doesn't have a topic
// (i.e., if it's a private message)
// - it's guaranteed to be present if the topic did indeed change
// - it will never be an empty string, because the server doesn't
// accept the empty string for a message's topic; it requires
// clients to specify something like `(no topic)` if no topic is
// desired.
orig_subject?: string,

orig_rendered_content: string,
prev_rendered_content_version: number,
rendered_content: string,
Expand Down
2 changes: 1 addition & 1 deletion src/message/messagesReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export default (state: MessagesState = initialState, action: Action): MessagesSt
subject_links: action.subject_links || oldMessage.subject_links,
edit_history: [
action.orig_rendered_content
? action.orig_subject
? action.orig_subject !== undefined
? {
prev_rendered_content: action.orig_rendered_content,
prev_subject: oldMessage.subject,
Expand Down

0 comments on commit e221fc1

Please sign in to comment.