Skip to content

Improve typing of normalizeRecipients #4122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rk-for-zulip
Copy link
Contributor

There's a TODO next to normalizeRecipients, so do it (fixing a minor bug in the process).

Also, add documentation for it. (The documentation is based on its original implementation and use in a241966, the comment noted in the commit. Its use has expanded beyond that... although not necessarily for the better.)

@rk-for-zulip
Copy link
Contributor Author

Withdrawn pending analysis of test failures.

@rk-for-zulip rk-for-zulip removed the request for review from gnprice May 20, 2020 22:34
... and, in so doing, fix a subtle bug.

There are fortunately only three callsites. Two of these would always
pass a list; the third, `matchRecipients`, might not.

This meant that if the name of a stream happened to be a user's
e-mail, `isMessageInNarrow` would incorrectly consider messages in
that stream to also be in the private-message narrow with that user.

Fix this by checking that a message passed to `matchRecipients` is of
type `"private"` before analyzing its `display_recipient`. Then
eliminate the relevant code.
@rk-for-zulip rk-for-zulip reopened this May 21, 2020
@rk-for-zulip rk-for-zulip force-pushed the normalize-recipients-typing branch from c0101d6 to 214c32d Compare May 21, 2020 00:44
@rk-for-zulip
Copy link
Contributor Author

(Test failure was due to the mock Message in the test missing fields. Revised.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant