Skip to content

Commit

Permalink
modelTypes: Split Message.stream_id into PmMessage and `StreamMes…
Browse files Browse the repository at this point in the history
…sage`.

In fact, just leave it out of `PmMessage` because it's absent on
PMs, as the comment says.

Flow gives us one substantive bit of feedback and one kind of false
alarm:

- We shouldn't be putting a `stream_id` on PMs in exampleData; so,
  stop doing that.

- When we start putting things together to call
  `addItemsToStreamArray`, we need to be sure `action.message` is a
  stream message. We were already sure of that; there's already an
  early return on `action.message.type !== stream`. But that early
  return needs to be just a bit later in order to satisfy Flow that
  `action.message` can't be a stream message. In particular, as Greg
  explains [1],

  """
  Specifically what's happening is: we check `action.message.type`,
  but then we call `getOwnUserId` -- and that's some function that
  might go and do anything. So in particular it might go and mutate
  the same object that we're looking at as `action.message`. So that
  object *had* had a `type` of `'private'`, but now it might not and
  so it might not have a `stream_id`.
  """

[1] zulip#4506 (comment)

 In particular, Flow
  sees the `getOwnUserId` call, and that's some function that might
  go and do anything -- including

But Flow seems
  to think it's done too early, and that something in the
  intervening code might thwart the job it's doing. So, move that
  early return a bit later.
  • Loading branch information
chrisbobbe committed Mar 10, 2021
1 parent d84ff96 commit 4a4be74
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 5 deletions.
1 change: 0 additions & 1 deletion src/__tests__/lib/exampleData.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ export const pmMessage = (args?: {|
// in real messages. (See comments on the Message type.)
display_recipient: recipients.map(displayRecipientFromUser),
id: randMessageId(),
stream_id: -1,
subject: '',
timestamp: 1556579160,
type: 'private',
Expand Down
4 changes: 2 additions & 2 deletions src/api/modelTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,6 @@ type MessageBase = $ReadOnly<{|
*/
display_recipient: string | $ReadOnlyArray<PmRecipientUser>, // `string` for type stream, else PmRecipientUser[]

stream_id: number, // FixMe: actually only for type `stream`, else absent.

subject: string,
subject_links: $ReadOnlyArray<string>,
|}>;
Expand All @@ -561,6 +559,8 @@ export type StreamMessage = $ReadOnly<{|

// TODO: Put stream-message fields here.
type: 'stream',

stream_id: number,
|}>;

/**
Expand Down
4 changes: 2 additions & 2 deletions src/unread/unreadStreamsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ import { getOwnUserId } from '../users/userSelectors';
const initialState: UnreadStreamsState = NULL_ARRAY;

const eventNewMessage = (state, action, globalState) => {
if (action.message.type !== 'stream') {
if (getOwnUserId(globalState) === action.message.sender_id) {
return state;
}

if (getOwnUserId(globalState) === action.message.sender_id) {
if (action.message.type !== 'stream') {
return state;
}

Expand Down

0 comments on commit 4a4be74

Please sign in to comment.